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. |
bsipocz
left a comment
There was a problem hiding this comment.
The test data file needs to be trimmed, otherwise this looks all OK.
Thanks!
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.
|
|
||
| - 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. |
5915fbe to
04fb9cc
Compare
bsipocz
left a comment
There was a problem hiding this comment.
I went ahead and rebased the PR to squash the commits about the new file together, so the big one doesn't end up in the git history.
Merging it now if CI is all green.
Thanks!
…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