Fix faling remote tests #3243
Fix faling remote tests #3243
Conversation
…require filter criteria, but it is a very large table and will take a long time to return if no filter criteria are given. Same is true for the skipped tests for kelttimeseries and superwasptimeseries. The error thrown for the keplertimeseries test was actually because no KEPID column exists; this should now be star_id. The 2nd fail was from an assumption that the cumulative table is not in TAP and would return a dict if asking to return the payload, which is no longer true -- now asserting string as expected. The 3rd failure was from a unit conversion warning for st_lumerr and is unrelated to testing the case sensitivity. Setting a selection will avoid tripping the test.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3243 +/- ##
=======================================
Coverage 69.07% 69.07%
=======================================
Files 232 232
Lines 19617 19617
=======================================
Hits 13551 13551
Misses 6066 6066 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CHANGES.rst
Outdated
| @@ -54,6 +54,8 @@ ipac.nexsci.nasa_exoplanet_archive | |||
|
|
|||
| - Fixed InvalidTableError for DI_STARS_EXEP and TD tables. [#3189] | |||
|
|
|||
| - Fixed failing remote tests. [#3243] | |||
There was a problem hiding this comment.
There is no need for a changelog for PRs like this (that only touches tests, docs, or internal restructuring that is not user facing).
It's just an FYI, you don't need to do anything about it, I'll remove the commit before merging this.
| NasaExoplanetArchive.query_criteria("keplertimeseries", kepid=8561063, quarter=14) | ||
| assert "'KEPID': invalid identifier" in str(error) | ||
| NasaExoplanetArchive.query_criteria("keplertimeseries", where="star_id=8561063") |
There was a problem hiding this comment.
so queries used to work where kepid was specified. Is that an API change now?
There was a problem hiding this comment.
I'm not sure exactly when that change was made, but yes, KEPID used to be a valid column. At least for that one the online docs are updated (https://exoplanetarchive.ipac.caltech.edu/docs/API_keplertimeseries_columns.html). For di_stars_exep the docs (https://exoplanetarchive.ipac.caltech.edu/docs/API_mission_stars.html) indicate there is a star_name column, but I’m not seeing any such column here: https://exoplanetarchive.ipac.caltech.edu/TAP/sync?query=select+*+from+DI_STARS_EXEP&format=csv
I've asked Doug to look into when these changes were made. For the latter, he thinks maybe the delivered data was just different than what the contributor had stated (so we should update the docs there, but might require reaching out to the contributor).
I will chat with Doug regarding the previous requirements for where-clauses when querying keplertimeseries, kelttimeseries, and superwasptimeseries (and possibly others), that disappeared when moving those tables over to TAP. We might want to implement some sort of warning for these large tables in TAP too, but it would require a broader discussion.
There was a problem hiding this comment.
Ok, I'll go ahead with the merge for this PR then, and if there are any more changes needed as a warning or enhancement, those can always come in a follow-up.
bsipocz
force-pushed
the
fix_exoarchive_failingtests
branch
from
fb2ae81 to
072b105
Compare
keplertimeseriesis now in TAP and does not require filter criteria, but it is a very large table and will take a long time to return if no filter criteria are given. Same is true for the skipped tests forkelttimeseriesandsuperwasptimeseries. The error thrown for thekeplertimeseriestest was actually because noKEPIDcolumn exists; this should now bestar_id. The 2nd fail was from an assumption that thecumulativetable is not in TAP and would return a dict if asking to return the payload, which is no longer true -- now asserting string as expected. The 3rd failure was from a unit conversion warning forst_lumerrand is unrelated to testing the case sensitivity. Setting a selection will avoid tripping the test.