Euclid: New remote tests#3407
Conversation
New remote tests for euclid
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3407 +/- ##
==========================================
- Coverage 70.51% 70.50% -0.01%
==========================================
Files 232 232
Lines 19997 19999 +2
==========================================
+ Hits 14100 14101 +1
- Misses 5897 5898 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bsipocz
left a comment
There was a problem hiding this comment.
Thank you!
There are a couple of comments that are blockers for the tests:
- one test is failing due to the dataset is in Angstrom --> we need to put those lines in a context manager for now and expect the warning
And could you please add back the code outputs into the docs as we should show the users what they can expect as outputs even if we don't test for exact matching.
| - New method, get_scientific_product_list, to retrieve scientific LE3 | ||
| products. [#3313] | ||
| - New cross-match method [#3386] | ||
| - New remote tests [#3407] |
There was a problem hiding this comment.
no need for changelog for non-user facing internals like this
There was a problem hiding this comment.
The entry referring to the PR has been removed.
| INFO: Parsing tables... [astroquery.utils.tap.core] | ||
| INFO: Done. [astroquery.utils.tap.core] | ||
| >>> print("Found", len(tables), "tables") # doctest: +IGNORE_OUTPUT | ||
| Found 34 tables |
There was a problem hiding this comment.
Why did you remove the print outputs here and below?
There was a problem hiding this comment.
Sorry, we were confused, since we though that the output should not be included for remote tests. We have copied them back.
There was a problem hiding this comment.
no worries. Local test runs will know better to skip these altogether so it won't cause problems for those, and keeping them in will be better for the user (or at least we think so)
| .. Skipping authentication requiring examples | ||
| .. doctest-skip:: | ||
| .. doctest-remote-data:: |
There was a problem hiding this comment.
so this example doesn't require authentication any more? It indeed seems to run for me locally except that we don’t get the expected outputs
| coord = SkyCoord(ra=265.8, dec=64.1, unit=(u.degree, u.degree), frame='icrs') | ||
| width = u.Quantity(0.1, u.deg) | ||
| height = u.Quantity(0.1, u.deg) | ||
| r = euclid.query_object(coordinate=coord, width=width, height=height, async_job=True, verbose=True) |
There was a problem hiding this comment.
This test fails with a vounit warning, I would put that into a pytest.warns context manager here knowing that we will be able to clean it up once upstream is changing behaviour
2298837 to
0f499ba
Compare
5008d84 to
9fe4042
Compare
EUCLIDSWRQ-247 New remote tests for euclid EUCLIDSWRQ-247 New remote tests for euclid EUCLIDSWRQ-247 Include PR number
bfde5c7 to
e48e283
Compare
bsipocz
left a comment
There was a problem hiding this comment.
I've pushed on more commit to fix all the doctest failures I saw locally.
Thanks for adding these tests!
fb505e1 to
f31df50
Compare
Dear Astroquery team,
in #3221 it was suggested to implement new remote tests for the Euclid module, since Q1 was out. We have included a new set of tests that eventually we will improve. We have also updated the documentation.
cc @esdc-esac-esa-int
jira: EUCLIDMNGT-1430