Skip to content

Conversation

daisycodes
Copy link
Contributor

This PR removes the use of from __future__ usages in the codebase. I'm using the 2to3 translator which comes with the python interpreter as a script. By default it uses a lot of fixers which made changes I wasn't sure is what we want. For example this line

print('Some text', someobject.name)

was replaced with

print(('Some text', someobject.name))

And also calls to the items() method of dictionaries were wrapped arounded the list() function. While all these are valid code, I just don't see the point and felt like these will just increase code review time. What are your thoughts on these modifications and is there another approach to removing all python2 code in a sensible way?

I'm not adding the closes section of this PR because I don't want the issue to be closed when this PR gets merged as it doesn't resolve the ticket completely.

@codecov
Copy link

codecov bot commented Oct 17, 2020

Codecov Report

Merging #1864 into master will decrease coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1864      +/-   ##
==========================================
- Coverage   64.44%   64.37%   -0.08%     
==========================================
  Files         200      200              
  Lines       16033    15998      -35     
==========================================
- Hits        10333    10298      -35     
  Misses       5700     5700              
Impacted Files Coverage Δ
astroquery/alfalfa/core.py 96.00% <ø> (-0.06%) ⬇️
astroquery/alma/core.py 33.97% <ø> (-0.14%) ⬇️
astroquery/astrometry_net/core.py 49.73% <ø> (-0.27%) ⬇️
astroquery/besancon/core.py 69.27% <ø> (-0.16%) ⬇️
astroquery/cosmosim/core.py 9.00% <ø> (-0.15%) ⬇️
astroquery/dace/core.py 82.50% <ø> (-0.43%) ⬇️
astroquery/esasky/core.py 32.88% <ø> (-0.19%) ⬇️
astroquery/eso/core.py 29.92% <ø> (-0.14%) ⬇️
astroquery/fermi/core.py 89.23% <ø> (-0.17%) ⬇️
astroquery/heasarc/core.py 21.48% <ø> (-0.65%) ⬇️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7ea01c...695297c. Read the comment docs.

@ceb8 ceb8 added this to the v0.4.2 milestone Oct 20, 2020
Copy link
Member

@ceb8 ceb8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Just need @bsipocz or @keflavich to take a last look at it and it should be ready to merge.

@bsipocz
Copy link
Member

bsipocz commented Oct 21, 2020

Thank you @daisycodes! I totally agree that changing the prints the 2to3 way makes little sense, so thanks for not pushing those to the PR.

@bsipocz bsipocz merged commit 83ebc45 into astropy:master Oct 21, 2020
@bsipocz bsipocz mentioned this pull request Oct 27, 2020
7 tasks
@eerovaher eerovaher mentioned this pull request Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment