Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix faling remote tests #3243

Merged
merged 1 commit into from
Mar 6, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ def test_invalid_query_exoplanets():


@pytest.mark.remote_data
def test_missing_criterion_kepler():
def test_invalid_query_kepler():
with pytest.raises(InvalidQueryError) as error:
NasaExoplanetArchive.query_criteria("keplertimeseries", where="kepid=8561063")
assert "Queries against the Kepler Time Series table require" in str(error)
NasaExoplanetArchive.query_criteria("keplertimeseries", kepid=8561063, quarter=14)
assert "'KEPID': invalid identifier" in str(error)
NasaExoplanetArchive.query_criteria("keplertimeseries", where="star_id=8561063")
Copy link
Member

Choose a reason for hiding this comment

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

so queries used to work where kepid was specified. Is that an API change now?

Copy link
Contributor Author

@rickynilsson rickynilsson Mar 6, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.



@pytest.mark.skip('TMP skip, server stuck with query')
Expand Down Expand Up @@ -135,10 +135,10 @@ def test_request_to_sql():

assert payload_sql == "select * from ps where hostname like 'Kepler%' order by hostname"

# "cumulative" table is not in TAP_TABLES, payload is sent directly as GET params
# "cumulative" table is now in TAP_TABLES
payload_dict = NasaExoplanetArchive.query_criteria(table="cumulative", where="pl_hostname like 'Kepler%'",
order="pl_hostname", get_query_payload=True)
assert isinstance(payload_dict, dict)
assert isinstance(payload_dict, str)


@pytest.mark.remote_data
Expand Down Expand Up @@ -190,5 +190,5 @@ def test_format():
@pytest.mark.remote_data
def test_table_case_sensivity():
# Regression test from #3090
table = NasaExoplanetArchive.query_criteria(table='DI_STARS_EXEP', select='*')
table = NasaExoplanetArchive.query_criteria(table='DI_STARS_EXEP', select='tic_id')
assert len(table) > 0
Loading