Simbad: refactor to use TAP
Hi astroquery,
[draft] Docs are not entirely clean yet, but the code should stay more or less like this before I un-draft. Here are some changes and questions about them:
List of changes:
Adding criteria
Formerly, there was only one way to enter criteria on the output, and it was the query_criteria method. This does not exist anymore, but is replaced by a new criteria argument in every query_*** method (except query_tap because people can directly write that into their ADQL string).
This new interface looks like this:
from astroquery.simbad import Simbad
Simbad.query_region("NGC 3540", radius="1d", criteria="otype != 'Galaxy..'")
<Table length=568>
main_id ra dec coo_err_maj coo_err_min coo_err_angle coo_wavelength coo_bibcode
deg deg mas mas deg
object float64 float64 float32 float32 int16 str1 object
------------------------------- ------------------ ------------------ ----------- ----------- ------------- -------------- -------------------
Gaia DR3 762197128415141632 166.97067113513333 36.64013738050528 0.0392 0.0617 90 O 2020yCat.1350....0G
Gaia DR3 761945928663296128 168.0368192975804 36.19277340338082 0.0493 0.0672 90 O 2020yCat.1350....0G
GRB 230606A 166.8095 36.484249999999996 2000.0 2000.0 -- X url:SWIFT
CLS 37 168.2657239642 35.90652522302 0.8057 0.9704 90 O 2020yCat.1350....0G
FIRST J110854.7+355104 167.22791666666666 35.851277777777774 570.0 510.0 0 1995ApJ...450..559B
FIRST J110740.1+364843 166.91745833333334 36.81213888888889 1090.0 1090.0 90 1995ApJ...450..559B
[TKK2018] 3605 167.24513 37.00419 -- -- -- O 2018A&A...618A..81T
[TKK2018] 3621 167.66022 36.88636 -- -- -- O 2018A&A...618A..81T
[T2015] nest 103154 167.92918 35.41882 -- -- -- O 2016A&A...588A..14T
Gaia DR3 762127451160945024 167.38398588803042 36.33682265764583 0.0545 0.0578 90 O 2020yCat.1350....0G
...
Where the criteria string can be translated from the old syntax to the new one with the helper class:
from astroquery.simbad.utils import CriteriaTranslator
CriteriaTranslator.parse("region(GAL,180 0,2d) & otype = 'G' & (galdim_minaxis >= 10|galdim_majaxis >= 10)")
"CONTAINS(POINT('ICRS', ra, dec), CIRCLE('ICRS', 86.40498828654475, 28.93617776179148, 2.0)) = 1 AND otype = 'G' AND (galdim_minaxis >= 10 OR galdim_majaxis >= 10)"
This could also be done automatically at each criteria insertion (like detecting if this is the old or new format and translating, maybe with a warning indicating the new ADQL syntax?). What are your thoughts?
On the output
Adding columns to the output
This was done with add_votable_fields before. This is replaced by add_to_output where the arguments are reproduced to fit as close as possible to the votable fields, but some were documented here but deprecated for years in Simbad. Seeing that there is no issue opened in this repo, these are not used.
That'd look like this:
- someone calling a votable field deprecated for years:
from astroquery.simbad import Simbad
simbad = Simbad()
simbad.add_to_output("einstein")
ValueError: 'einstein' is no longer a part of SIMBAD. It was moved into a separate VizieR catalog. It is possible to query it with the `astroquery.vizier` module.
- someone calling with a column/table that has a different name between the old and new interfaces
from astroquery.simbad import Simbad
simbad = Simbad()
simbad.add_to_output("id(1)")
<stdin>:1: DeprecationWarning: 'id(1)' has been renamed 'main_id'. You'll see it appearing with its new name in the output table
Every other possibility can be listed with:
from astroquery.simbad import Simbad
Simbad.list_output_options()
<Table length=97>
name description type
object object object
----------------- ---------------------------------------------------------------------------- -----------------------
ids all names concatenated with pipe table
otypedef all names and definitions for the object types table
ident Identifiers of an astronomical object table
flux Magnitude/Flux information about an astronomical object table
allfluxes all flux/magnitudes U,B,V,I,J,H,K,u_,g_,r_,i_,z_ table
has_ref Associations between astronomical objects and their bibliographic references table
mesDistance Collection of distances (pc, kpc or Mpc) by several means. table
...
(note that the number of possible options went from 107 to 97, among with 12 tables that really don't exist in Simbad since years. So the possibilities are now slightly bigger :slightly_smiling_face: )
Changes in output style
- As hinted above, some columns don't have the same name in the two Simbad interfaces. They are very close though.
- Unfortunately, the votable output of Simbad-TAP has lower case column names. This is a breaking change from previous interface.
- The default output format for coordinates is degrees rather than sexadecimal like before.
All these could be hidden to the users by modifying the output table in query_tap on python side. Is it worth it?
Empty result
Empty result is valid in TAP. So the default ROW_LIMIT could not stay at zero to mean infinity. I copied VizieR module API and went with -1.
It also means that a query with no answer returns an empty table and not None like before. This broke JWST module, see later is this long PR description.
Caching
Caching in the simbad module is now handled by python built-in lru_cache. This might be reverted if we add a caching mechanism to BaseVOQuery (something that'd keep things in a votable-xml format in the default astroquery cache folder maybe?). But I did not go all this way yet.
Changes to query_*** methods (except the criteria argument covered above)
Query_object
The script number ID in query_object is replaced by matched_id that contains the ID that corresponded to the wildcard expression.
It looks like this:
from astroquery.simbad import Simbad
simbad = Simbad()
simbad.add_to_output("otype")
simbad.ROW_LIMIT = 20
eso_clusters = simbad.query_object("ESO*", wildcard=True, criteria="otype='Cl*..'")
eso_clusters["main_id", "ra", "dec", "otype", "matched_id"]
<Table length=20>
main_id ra dec otype matched_id
deg deg
object float64 float64 object object
------------------ ------------------ ------------------- ------ -----------
NGC 1776 74.66475 -66.42954166666667 Cl* ESO 85-28
ESO 57-81 96.2041666666667 -71.65833333333335 Cl* ESO 57-81
NGC 2097 86.02948 -62.78351 Cl* ESO 86-28
NGC 1865 78.10391666666668 -68.77200277777777 Cl* ESO 56-78
NGC 299 13.353083333333332 -72.19655555555556 Cl* ESO 51-5
NGC 1849 77.39450000000001 -66.31465833333334 Cl* ESO 85-49
NGC 1826 76.39174999999999 -66.22904166666666 Cl* ESO 85-43
ESO 85-41 76.35151 -63.28754 Cl* ESO 85-41
NGC 2121 87.05495833333332 -71.48111111111112 Cl* ESO 57-40
NGC 2019 82.98533333333333 -70.15902777777778 Cl* ESO 56-145
NGC 1777 73.95 -74.28333333333333 Cl* ESO 33-1
NGC 2047 83.98316666666666 -70.18519722222223 Cl* ESO 56-167
Cl Westerlund 1 251.76 -45.852000000000004 Cl* ESO 277-12
Cl Terzan 5 267.02083333333337 -24.779999999999998 Cl* ESO 520-27
ESO 51-3 12.208333333333334 -69.86999999999999 Cl* ESO 51-3
NGC 2257 97.55258333333333 -64.32777777777777 Cl* ESO 87-24
NGC 2203 91.17570833333333 -75.43772222222222 Cl* ESO 34-4
NGC 1751 73.55379166666667 -69.80744444444444 Cl* ESO 56-23
NGC 416 16.995958333333334 -72.35569444444444 Cl* ESO 29-32
NGC 411 16.98183333333333 -71.76752777777777 Cl* ESO 51-19
Query_objects
It now has a typed_id column as requested in #967 . The object_number_id replaces script_number_id (but this could be reverted, it's just strange as there is really one script)
Looks like this:
from astroquery.simbad import Simbad
Simbad.query_objects(("m1", "m2", "m503", "m21"))
<Table length=4>
main_id ra dec coo_err_maj coo_err_min coo_err_angle coo_wavelength coo_bibcode typed_id object_number_id
deg deg mas mas deg
object float64 float64 float32 float32 int16 str1 object object int64
------- ------------------ ------------------- ----------- ----------- ------------- -------------- ------------------- -------- ----------------
M 1 83.6287 22.0147 18500.0 18500.0 0 R 1995AuJPh..48..143S m1 1
M 2 323.36258333333336 -0.8232499999999998 -- -- -- O 2010AJ....140.1830G m2 2
-- -- -- -- -- m503 3
M 21 271.036 -22.505000000000003 -- -- -- O 2021A&A...647A..19T m21 4
Note that the requested feature (I could not find the issue anymore) that objects not found would return an empty line is there: M503 obviously does not exist.
Query_bibcode
The output is very different.
Former output:
from astroquery.simbad import Simbad
Simbad.query_bibcode("1894MNRAS..55...17L")
<Table length=1>
References
str132
---------------------------------------------------------------------------------------------------------------------------------------
1894MNRAS..55...17L = DOI 10.1093/mnras/55.1.17\nMon. Not. R. Astron. Soc., 55, 17 (1894)\nLEWIS T.\nNote on the orbit of kappa Pegasi.
New output:
from astroquery.simbad import Simbad
Simbad.query_bibcode("1894MNRAS..55...17L")
<Table length=1>
bibcode doi journal nbobject page last_page title volume year
object object object int32 int32 int32 object int32 int16
------------------- --------------------- ------- -------- ----- --------- ---------------------------------- ------ -----
1894MNRAS..55...17L 10.1093/mnras/55.1.17 MNRAS 1 17 -- Note on the orbit of kappa Pegasi. 55 1894
I know it's a very different output but I really hated the former one. I can try to reproduce the old one but :frowning_face:
Also, it is now possible to retrieve the abstract with abstract=True.
Query_region, query_catalog, query_bibobj, query_object_ids
No big changes
In JWST
As Simbad is used in the tests, I just patched what I could how I could, may not be the prettiest way to go :/
Also, now an empty response returns an empty table and not None, so I reflected that in jwst core.py.
Issues linked to this PR
Fixes: #2198 Fixes: #1468 Partially: #967 (It is done for query_objects, but not for query_region yet)
Hello @ManonMarchand! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
- In the file
astroquery/simbad/criteria_lextab.py:
Line 13:12: E221 multiple spaces before operator Line 14:11: E221 multiple spaces before operator Line 15:12: E221 multiple spaces before operator Line 16:13: E221 multiple spaces before operator Line 18:12: E221 multiple spaces before operator Line 18:121: E501 line too long (498 > 120 characters)
- In the file
astroquery/simbad/criteria_parsetab.py:
Line 20:121: E501 line too long (458 > 120 characters) Line 21:1: W293 blank line contains whitespace Line 22:24: E231 missing whitespace after ':' Line 22:28: E231 missing whitespace after ',' Line 22:30: E231 missing whitespace after ',' Line 22:32: E231 missing whitespace after ',' Line 22:34: E231 missing whitespace after ',' Line 22:36: E231 missing whitespace after ',' Line 22:39: E231 missing whitespace after ',' Line 22:41: E231 missing whitespace after ',' Line 22:43: E231 missing whitespace after ',' Line 22:45: E231 missing whitespace after ',' Line 22:48: E231 missing whitespace after ',' Line 22:57: E231 missing whitespace after ':' Line 22:61: E231 missing whitespace after ',' Line 22:63: E231 missing whitespace after ',' Line 22:65: E231 missing whitespace after ',' Line 22:67: E231 missing whitespace after ',' Line 22:69: E231 missing whitespace after ',' Line 22:71: E231 missing whitespace after ',' Line 22:74: E231 missing whitespace after ',' Line 22:76: E231 missing whitespace after ',' Line 22:78: E231 missing whitespace after ',' Line 22:80: E231 missing whitespace after ',' Line 22:83: E231 missing whitespace after ',' Line 22:86: E231 missing whitespace after ',' Line 22:95: E231 missing whitespace after ':' Line 22:99: E231 missing whitespace after ',' Line 22:101: E231 missing whitespace after ',' Line 22:103: E231 missing whitespace after ',' Line 22:105: E231 missing whitespace after ',' Line 22:107: E231 missing whitespace after ',' Line 22:110: E231 missing whitespace after ',' Line 22:112: E231 missing whitespace after ',' Line 22:114: E231 missing whitespace after ',' Line 22:116: E231 missing whitespace after ',' Line 22:119: E231 missing whitespace after ',' Line 22:121: E501 line too long (598 > 120 characters) Line 22:126: E231 missing whitespace after ':' Line 22:130: E231 missing whitespace after ',' Line 22:132: E231 missing whitespace after ',' Line 22:135: E231 missing whitespace after ',' Line 22:138: E231 missing whitespace after ',' Line 22:141: E231 missing whitespace after ',' Line 22:144: E231 missing whitespace after ',' Line 22:147: E231 missing whitespace after ',' Line 22:150: E231 missing whitespace after ',' Line 22:153: E231 missing whitespace after ',' Line 22:156: E231 missing whitespace after ',' Line 22:159: E231 missing whitespace after ',' Line 22:161: E231 missing whitespace after ',' Line 22:164: E231 missing whitespace after ',' Line 22:168: E231 missing whitespace after ',' Line 22:171: E231 missing whitespace after ',' Line 22:174: E231 missing whitespace after ',' Line 22:177: E231 missing whitespace after ',' Line 22:180: E231 missing whitespace after ',' Line 22:183: E231 missing whitespace after ',' Line 22:186: E231 missing whitespace after ',' Line 22:189: E231 missing whitespace after ',' Line 22:192: E231 missing whitespace after ',' Line 22:195: E231 missing whitespace after ',' Line 22:198: E231 missing whitespace after ',' Line 22:202: E231 missing whitespace after ':' Line 22:206: E231 missing whitespace after ',' Line 22:208: E231 missing whitespace after ',' Line 22:210: E231 missing whitespace after ',' Line 22:213: E231 missing whitespace after ',' Line 22:216: E231 missing whitespace after ',' Line 22:219: E231 missing whitespace after ',' Line 22:222: E231 missing whitespace after ',' Line 22:225: E231 missing whitespace after ',' Line 22:228: E231 missing whitespace after ',' Line 22:231: E231 missing whitespace after ',' Line 22:234: E231 missing whitespace after ',' Line 22:237: E231 missing whitespace after ',' Line 22:239: E231 missing whitespace after ',' Line 22:242: E231 missing whitespace after ',' Line 22:246: E231 missing whitespace after ',' Line 22:248: E231 missing whitespace after ',' Line 22:250: E231 missing whitespace after ',' Line 22:252: E231 missing whitespace after ',' Line 22:255: E231 missing whitespace after ',' Line 22:258: E231 missing whitespace after ',' Line 22:261: E231 missing whitespace after ',' Line 22:264: E231 missing whitespace after ',' Line 22:267: E231 missing whitespace after ',' Line 22:270: E231 missing whitespace after ',' Line 22:273: E231 missing whitespace after ',' Line 22:276: E231 missing whitespace after ',' Line 22:280: E231 missing whitespace after ':' Line 22:284: E231 missing whitespace after ',' Line 22:286: E231 missing whitespace after ',' Line 22:288: E231 missing whitespace after ',' Line 22:291: E231 missing whitespace after ',' Line 22:294: E231 missing whitespace after ',' Line 22:297: E231 missing whitespace after ',' Line 22:300: E231 missing whitespace after ',' Line 22:303: E231 missing whitespace after ',' Line 22:306: E231 missing whitespace after ',' Line 22:309: E231 missing whitespace after ',' Line 22:312: E231 missing whitespace after ',' Line 22:315: E231 missing whitespace after ',' Line 22:317: E231 missing whitespace after ',' Line 22:320: E231 missing whitespace after ',' Line 22:324: E231 missing whitespace after ',' Line 22:326: E231 missing whitespace after ',' Line 22:328: E231 missing whitespace after ',' Line 22:330: E231 missing whitespace after ',' Line 22:333: E231 missing whitespace after ',' Line 22:336: E231 missing whitespace after ',' Line 22:339: E231 missing whitespace after ',' Line 22:342: E231 missing whitespace after ',' Line 22:345: E231 missing whitespace after ',' Line 22:348: E231 missing whitespace after ',' Line 22:351: E231 missing whitespace after ',' Line 22:354: E231 missing whitespace after ',' Line 22:372: E231 missing whitespace after ':' Line 22:376: E231 missing whitespace after ',' Line 22:378: E231 missing whitespace after ',' Line 22:381: E231 missing whitespace after ',' Line 22:384: E231 missing whitespace after ',' Line 22:389: E231 missing whitespace after ':' Line 22:393: E231 missing whitespace after ',' Line 22:395: E231 missing whitespace after ',' Line 22:398: E231 missing whitespace after ',' Line 22:401: E231 missing whitespace after ',' Line 22:408: E231 missing whitespace after ':' Line 22:412: E231 missing whitespace after ',' Line 22:414: E231 missing whitespace after ',' Line 22:418: E231 missing whitespace after ',' Line 22:421: E231 missing whitespace after ',' Line 22:431: E231 missing whitespace after ':' Line 22:435: E231 missing whitespace after ',' Line 22:437: E231 missing whitespace after ',' Line 22:441: E231 missing whitespace after ',' Line 22:444: E231 missing whitespace after ',' Line 22:448: E231 missing whitespace after ':' Line 22:452: E231 missing whitespace after ',' Line 22:454: E231 missing whitespace after ',' Line 22:457: E231 missing whitespace after ',' Line 22:460: E231 missing whitespace after ',' Line 22:463: E231 missing whitespace after ',' Line 22:466: E231 missing whitespace after ',' Line 22:469: E231 missing whitespace after ',' Line 22:472: E231 missing whitespace after ',' Line 22:475: E231 missing whitespace after ',' Line 22:478: E231 missing whitespace after ',' Line 22:481: E231 missing whitespace after ',' Line 22:483: E231 missing whitespace after ',' Line 22:488: E231 missing whitespace after ',' Line 22:491: E231 missing whitespace after ',' Line 22:494: E231 missing whitespace after ',' Line 22:497: E231 missing whitespace after ',' Line 22:500: E231 missing whitespace after ',' Line 22:503: E231 missing whitespace after ',' Line 22:506: E231 missing whitespace after ',' Line 22:509: E231 missing whitespace after ',' Line 22:512: E231 missing whitespace after ',' Line 22:515: E231 missing whitespace after ',' Line 22:518: E231 missing whitespace after ',' Line 22:521: E231 missing whitespace after ',' Line 22:530: E231 missing whitespace after ':' Line 22:534: E231 missing whitespace after ',' Line 22:537: E231 missing whitespace after ',' Line 22:540: E231 missing whitespace after ',' Line 22:542: E231 missing whitespace after ',' Line 22:546: E231 missing whitespace after ',' Line 22:549: E231 missing whitespace after ',' Line 22:552: E231 missing whitespace after ',' Line 22:555: E231 missing whitespace after ',' Line 22:564: E231 missing whitespace after ':' Line 22:568: E231 missing whitespace after ',' Line 22:570: E231 missing whitespace after ',' Line 22:574: E231 missing whitespace after ',' Line 22:577: E231 missing whitespace after ',' Line 22:584: E231 missing whitespace after ':' Line 22:588: E231 missing whitespace after ',' Line 22:590: E231 missing whitespace after ',' Line 22:594: E231 missing whitespace after ',' Line 22:597: E231 missing whitespace after ',' Line 26:4: E111 indentation is not a multiple of four Line 26:10: E231 missing whitespace after ',' Line 26:26: E231 missing whitespace after ',' Line 27:7: E111 indentation is not a multiple of four Line 27:10: E713 test for membership should be 'not in' Line 27:30: E701 multiple statements on one line (colon) Line 27:31: E241 multiple spaces after ':' Line 28:7: E111 indentation is not a multiple of four Line 31:29: E231 missing whitespace after ':' Line 31:33: E231 missing whitespace after ',' Line 31:35: E231 missing whitespace after ',' Line 31:37: E231 missing whitespace after ',' Line 31:39: E231 missing whitespace after ',' Line 31:41: E231 missing whitespace after ',' Line 31:44: E231 missing whitespace after ',' Line 31:46: E231 missing whitespace after ',' Line 31:49: E231 missing whitespace after ',' Line 31:52: E231 missing whitespace after ',' Line 31:55: E231 missing whitespace after ',' Line 35:4: E111 indentation is not a multiple of four Line 36:8: E111 indentation is not a multiple of four Line 36:11: E713 test for membership should be 'not in' Line 36:29: E701 multiple statements on one line (colon) Line 37:8: E111 indentation is not a multiple of four Line 40:3: E121 continuation line under-indented for hanging indent Line 40:20: E231 missing whitespace after ',' Line 40:25: E231 missing whitespace after ',' Line 40:27: E231 missing whitespace after ',' Line 40:32: E231 missing whitespace after ',' Line 40:37: E231 missing whitespace after ',' Line 41:37: E231 missing whitespace after ',' Line 41:48: E231 missing whitespace after ',' Line 41:50: E231 missing whitespace after ',' Line 41:66: E231 missing whitespace after ',' Line 41:77: E231 missing whitespace after ',' Line 42:37: E231 missing whitespace after ',' Line 42:48: E231 missing whitespace after ',' Line 42:50: E231 missing whitespace after ',' Line 42:67: E231 missing whitespace after ',' Line 42:78: E231 missing whitespace after ',' Line 43:30: E231 missing whitespace after ',' Line 43:41: E231 missing whitespace after ',' Line 43:43: E231 missing whitespace after ',' Line 43:68: E231 missing whitespace after ',' Line 43:79: E231 missing whitespace after ',' Line 44:47: E231 missing whitespace after ',' Line 44:58: E231 missing whitespace after ',' Line 44:60: E231 missing whitespace after ',' Line 44:80: E231 missing whitespace after ',' Line 44:91: E231 missing whitespace after ',' Line 45:47: E231 missing whitespace after ',' Line 45:58: E231 missing whitespace after ',' Line 45:60: E231 missing whitespace after ',' Line 45:80: E231 missing whitespace after ',' Line 45:91: E231 missing whitespace after ',' Line 46:32: E231 missing whitespace after ',' Line 46:43: E231 missing whitespace after ',' Line 46:45: E231 missing whitespace after ',' Line 46:65: E231 missing whitespace after ',' Line 46:76: E231 missing whitespace after ',' Line 47:47: E231 missing whitespace after ',' Line 47:58: E231 missing whitespace after ',' Line 47:60: E231 missing whitespace after ',' Line 47:89: E231 missing whitespace after ',' Line 47:100: E231 missing whitespace after ',' Line 48:36: E231 missing whitespace after ',' Line 48:47: E231 missing whitespace after ',' Line 48:49: E231 missing whitespace after ',' Line 48:67: E231 missing whitespace after ',' Line 48:78: E231 missing whitespace after ',' Line 49:39: E231 missing whitespace after ',' Line 49:50: E231 missing whitespace after ',' Line 49:52: E231 missing whitespace after ',' Line 49:73: E231 missing whitespace after ',' Line 49:84: E231 missing whitespace after ',' Line 50:24: E231 missing whitespace after ',' Line 50:35: E231 missing whitespace after ',' Line 50:37: E231 missing whitespace after ',' Line 50:57: E231 missing whitespace after ',' Line 50:68: E231 missing whitespace after ','
Comment last updated at 2024-06-20 22:18:08 UTC
On codestyle:
tox -e codestyle tells me:
❯ tox -e codestyle
codestyle: commands[0]> flake8 astroquery --count
astroquery/simbad/tests/test_deprecated_simbad.py:482:78: W504 line break after binary operator
and here I should break before? Which one do you prefer? I'm happy with both.
Edit: I applied the line breaking after operator rule to reduce the number of issues with the PEP8 detector here.
On criteria_lextab.py and criteria_parser.py
These files are automatically generated by the astroquery.utils.parsing tools (lex and yacc) so I'd think I shouldn't edit them?
Similar files are not linted in astropy code base ex: https://github.com/astropy/astropy/blob/main/astropy/units/format/generic_parsetab.py
Codecov Report
Attention: Patch coverage is 97.12526% with 14 lines in your changes missing coverage. Please review.
Project coverage is 67.52%. Comparing base (
0ffd5d4) to head (98ae3b4). Report is 55 commits behind head on main.
:exclamation: Current head 98ae3b4 differs from pull request most recent head 6594ebb
Please upload reports for the commit 6594ebb to get more accurate results.
| Files | Patch % | Lines |
|---|---|---|
| astroquery/simbad/core.py | 95.95% | 12 Missing :warning: |
| astroquery/simbad/setup_package.py | 33.33% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #2954 +/- ##
==========================================
+ Coverage 67.17% 67.52% +0.34%
==========================================
Files 231 233 +2
Lines 18247 18224 -23
==========================================
+ Hits 12258 12305 +47
+ Misses 5989 5919 -70
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
On the oldest versions for all dependencies failure:
I thought yacc and lex just changed places between astropy 4.2.1 and astropy 4.3, but some method's signatures changed too, see commit here https://github.com/astropy/astropy/commit/85bffd9f0241bcbbc70c6b81c4b2281a5ed55bc6
The currently minimal astropy version supported in astroquery is 4.2.1. Is the jump negotiable? I don't see a lot of deprecations in the change log between these two versions.
The currently minimal astropy version supported in astroquery is 4.2.1. Is the jump negotiable? I don't see a lot of deprecations in the change log between these two versions.
This won't make the cut for 0.4.7, and after that one is out I'm planning to bump the minimum required versions (at least to astropy 5.0, but maybe even something newer), so no need to do workaround for support for old versions.
astroquery/simbad/tests/test_deprecated_simbad.py:482:78: W504 line break after binary operator and here I should break before? Which one do you prefer? I'm happy with both.
We enforce W504 and ignore W503 as break before operator is a bit more legible logic.
Hello!
I'm almost in a un-draft state :)
Here are the remaining points that I'm unsure about:
- which version of astroquery do you think this should be merged in? Is there a way to leave an extended changes description somewhere? Like a more narrative changelog?
- what did I do wrong with the docs? I don't have failures locally (only warnings about other modules)
Sorry for the delay in responding:
which version of astroquery do you think this should be merged in? Is there a way to leave an extended changes description somewhere? Like a more narrative changelog?
Next release, 0.4.8. At some point I'll refactor the versioning and related infrastructure, but don't have an ETA when it happens. For a more detailed narrative about the changes I would suggest to add a section in the narrative documentation. In practice that is the place users will notice features, I doubt that many refer to the changelog to obtain extended descriptions.
what did I do wrong with the docs? I don't have failures locally (only warnings about other modules)
I suspect this also run into the same pyvo related RTD failure and was unrelated to your changes.
Switching back to draft, I'm tweaking a bit the support of the legacy 'query_criteria'
Hi all! Sorry for the long delay, this should be ready to review. The description on top of the PR is updated to this new state
Sorry for the long delay, this should be ready to review. The description on top of the PR is updated to this new state
I'm still out at a workshop and working towards a deadline this week so it will take a bit of time to get back to this.
@keflavich : I think all your review comments are addressed? @bsipocz : no worries :slightly_smiling_face:
I found out that the wildcards in query_objects did not work. This is fixed now, but the remote test is so slow that I marked it to be skipped with the SKIP_SLOW boolean, like what the Alma module does for its own slow tests.
I'm not sure that supporting the wildcards in this method is absolutely necessary, or maybe I don't get the use case. Anyhow, it works now for patient people.
I think all your comments are addressed except the ones about parameters deprecation. What is the correct way of doing?
I think all your comments are addressed except the ones about parameters deprecation. What is the correct way of doing?
As I see you addressed them (most was already done in the code), it's just the changelog that needs a bit of rephrasing.