Skip to content

Conversation

@eerovaher
Copy link
Member

The exception handling in astropy.coordinates changed in version 5.0. This pull request ensures astroquery.utils.commons.parse_coordinates() works with astropy 5.0 and also with the older versions.

Fixes #2194.

The exception handling in `astropy.coordinates` changed in version 5.0.
This commit ensures `astroquery.utils.commons.parse_coordinates()` works
with `astropy` 5.0 and also with the older versions.
@codecov
Copy link

codecov bot commented Nov 8, 2021

Codecov Report

Merging #2196 (34a72df) into main (75cfd69) will increase coverage by 0.01%.
The diff coverage is 94.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2196      +/-   ##
==========================================
+ Coverage   66.10%   66.11%   +0.01%     
==========================================
  Files         418      418              
  Lines       28167    28180      +13     
==========================================
+ Hits        18619    18631      +12     
- Misses       9548     9549       +1     
Impacted Files Coverage Δ
astroquery/query.py 58.87% <80.00%> (+0.46%) ⬆️
astroquery/simbad/core.py 89.75% <100.00%> (+0.09%) ⬆️
astroquery/simbad/tests/test_simbad.py 98.35% <100.00%> (+0.02%) ⬆️
astroquery/utils/commons.py 78.78% <100.00%> (+0.10%) ⬆️

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 f18f02b...34a72df. Read the comment docs.

@keflavich
Copy link
Contributor

Looks like this solves #2194?

@keflavich
Copy link
Contributor

@bsipocz I'm pretty sure this does solve the broken CI issues.

@eerovaher thank you!

@bsipocz bsipocz added this to the v0.4.4 milestone Nov 8, 2021
@bsipocz bsipocz merged commit bc722c9 into astropy:main Nov 8, 2021
@bsipocz
Copy link
Member

bsipocz commented Nov 8, 2021

Thank you @eerovaher!

@astrofrog
Copy link
Member

@bsipocz @keflavich - just to check, are you planning a release of astroquery soon? If possible I would like to hold back from releasing astropy 5.0 until there is acompatible astroquery version so that users have a pathway to upgrade.

@astrofrog
Copy link
Member

Note that this still fails with 5.0rc2, I think because minversion doesn't include release candidates. So I think we are ok with 5.0rc2, but I wonder if it would be more robust to check the length of args instead of relying on the astropy version.

@bsipocz
Copy link
Member

bsipocz commented Nov 11, 2021

Yes, hopefully today, if not, then by the end of the week for sure.

@bsipocz
Copy link
Member

bsipocz commented Nov 11, 2021

If you or someone else could open a PR that doesn't check the version, please do so. I have a lot of smallish PRs that needs to be done for the release, so this is not at the top of my personal list.

@eerovaher
Copy link
Member Author

Note that this still fails with 5.0rc2, I think because minversion doesn't include release candidates. So I think we are ok with 5.0rc2, but I wonder if it would be more robust to check the length of args instead of relying on the astropy version.

An alternative would be to replace

ASTROPY_LT_5_0 = not minversion('astropy', '5.0')

with

 ASTROPY_LT_5_0 = not minversion('astropy', '5.0rc1')

@eerovaher eerovaher deleted the fix-parse-coordinates branch November 25, 2021 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

astropy feature freeze broke CI

4 participants