GAIA: include table size in the class TapTableMeta #2970
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2970 +/- ##
=======================================
Coverage 66.81% 66.81%
=======================================
Files 237 237
Lines 18319 18324 +5
=======================================
+ Hits 12239 12244 +5
Misses 6080 6080 ☔ View full report in Codecov by Sentry. |
| @@ -74,11 +104,13 @@ def test_job_results_parser(): | |||
| file.close() | |||
|
|
|||
|
|
|||
| def __check_table(table, baseName, numColumns, columnsData): | |||
| qualifiedName = f"public.{baseName}" | |||
| def __check_table(table, qualifiedName, numColumns, columnsData, size_bytes=-1): | |||
There was a problem hiding this comment.
It's inconsequential here as this is private and in the tests, but generally, we do go with None defaults rather than something custom invalid filler, e.g. -999, or -1 as here, etc.
There was a problem hiding this comment.
I agree with you. I've updated the default value to None.
There was a problem hiding this comment.
Is it necessary to add such a big file for the purpose of one reasonably straightforward test? (it's 10+% of the size of the whole package). Could you trim it back to contain hardly much more than what the tests actually use from it, or come up with a similar dummy file? (and then we need to overwrite the history, so it's not entering in the git history either)
There was a problem hiding this comment.
I trimmed the file, since the entire file was not required for the test.
CHANGES.rst
Outdated
| @@ -211,6 +211,9 @@ gaia | |||
|
|
|||
| - Default Gaia catalog updated to DR3. [#2596] | |||
|
|
|||
| - Include table size in the class TapTableMeta returned by the functions load_tables and load_table, in the class Tap. | |||
There was a problem hiding this comment.
changelog needs to be moved to the new release section
There was a problem hiding this comment.
I don't understand where should I include this information.
There was a problem hiding this comment.
There is an almost empty 0.4.8 section at the very top of the file, but I can move this one (and those from your other PRs right before merging them)
There was a problem hiding this comment.
Thanks for you answer. I included the information for the PR in the section you told me.
|
We have reviewed the comments in #2863 and we would like to use this PR to update the documentation for the functions load_table and load_tables. |
bsipocz
force-pushed
the
ESA_gaia_include_table_size_in_TapTableMeta
branch
from
5915fbe to
04fb9cc
Compare
…ead_http_response_GAIAPCR-1308 GAIA fix the method read_http_response to retrieve json files recently included in #2970
User tables do not display information on their physical size on disk. This makes account of the user quota difficult. That attribute should be visible to Astroquery users.
A new test was made.
cc @esdc-esac-esa-int
Jira: GAIAPCR-1036