[EUCLIDPCR-2075] update get_product_list get_scientific_product_list …#3599
Conversation
…get_tile_catalogue_list methods for TAP 10 array handling
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3599 +/- ##
==========================================
- Coverage 73.21% 73.18% -0.03%
==========================================
Files 226 226
Lines 21053 21030 -23
==========================================
- Hits 15414 15391 -23
Misses 5639 5639 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Bumps the actions group with 2 updates: [actions/checkout](https://github.com/actions/checkout) and [codecov/codecov-action](https://github.com/codecov/codecov-action). Updates `actions/checkout` from 6.0.2 to 6.0.3 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@de0fac2...df4cb1c) Updates `codecov/codecov-action` from 6.0.0 to 6.0.1 - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@57e3a13...e79a696) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 6.0.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: codecov/codecov-action dependency-version: 6.0.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] <support@github.com>
…_list-get_scientific_product_list-get_tile_catalogue_list-methods
|
Hello, We have been investigating this issue and ran the following test: @pytest.mark.remote_data When executing it, we get the following error: From our analysis (with @cosmoJFH ), this seems to be related to how astropy handles arrays of strings when generating VOTables. It looks like a potential bug or limitation in the library rather than an issue in our implementation. Given this, we think it would be safer not to merge these changes yet until we better understand the root cause. For that reason, we are marking this PR as Draft to block the merge for now. @bsipocz , do you have any insight into this? Thanks! |
|
@Enriquerzrz - is this a new test you try to add? I don't see any related failures when running the remote tests locally. That being said, something gone wrong with the rebase, unrelated commits are now picked up as well as there is a merge commit. I'm happy to rebase and then merge though, let me know the answer about the test |
bsipocz
left a comment
There was a problem hiding this comment.
The diff looks good, tests pass for me when I run them locally, but let's hold up until we see if the traceback mentioned in the comment is coming from an old test or something not yet committed.
| - Removed unnecessary CAST(... AS text) from array columns in ``get_product_list``and | ||
| ``__get_tile_catalogue_list``, as TAP 10.x now returns proper arrays. [#3599] |
There was a problem hiding this comment.
This is all enternal cleanup, so it doesn't need the changelog. I'll clean up the changelog file before release.
|
Hello @bsipocz The test included in the comment has not been merged. It is a local test that we added afterwards when my colleague identified the issue, but it is not part of the branch. |
|
To me it looks like that the response for this query is empty. So maybe that case needs to be covered in the code? |
The regression is an uncommitted test, so let's see it fixed before merging this.
|
We carried out the following test. First, we saved the VOTable obtained using the get_product_list method: ( The error is described in https://docs.astropy.org/en/latest/io/votable/api_exceptions.html#e01-invalid-size-specifier-x-for-a-char-unicode-field-in-field-y Also see astropy/astropy#17098 |
Jira: EUCLIDPCR-2075
CC: @esdc-esac-esa-int