-
Notifications
You must be signed in to change notification settings - Fork 27
Add file size columns to download status table (#249) #272
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
base: master
Are you sure you want to change the base?
Conversation
Test verifies animint.js generates total_MB and mean_MB columns in download status table. Currently fails, demonstrating these size columns are not present in the table. Addresses #249
|
Add download size info to status table (#249) Adds total_MB, mean_MB, and rows columns to the download status table. Browser screenshot verification included showing new columns working. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #272 +/- ##
==========================================
- Coverage 72.73% 69.57% -3.17%
==========================================
Files 164 163 -1
Lines 8792 6014 -2778
==========================================
- Hits 6395 4184 -2211
+ Misses 2397 1830 -567
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Calculate file sizes using file.size() in saveChunks() - Store chunk_info (bytes, rows) in plot.json - Update table cells after download (not in draw_geom) - Remove 36-line JavaScript calculation loop
|
No obvious timing issues in HEAD=add-download-size-info Generated via commit db9139b Download link for the artifact containing the test results: ↓ atime-results.zip
|
|
Thanks! This is a good start. I see the result below using https://github.com/animint/animint2/blob/master/inst/examples/WorldBank-paper.R Can you please make the following changes?
|
- Remove 'selected chunk', 'status', and 'mean MB' columns - Add combined 'files', 'MB', and 'rows' columns (downloaded/total format) - Right-justify all numeric columns - Fix common chunk tracking: count in both downloaded and total counts - Track size (bytes/rows) for common chunks in R code - Filter chunk_info to only include each geom's own chunks - Count only .tsv files for accurate totals in JavaScript - Update tests to verify new column structure
Use 1048576 (1024*1024) consistently for MB conversion instead of mixing 1e6 (1000000) and 1048576. This ensures consistent size reporting for both common chunks and regular chunks in the download status table.
Switch test-download-status-table.R to validate rendered HTML instead of
the JS source. Verify headers and right-justified numeric columns via XPath.
Update test-renderer1-variable-value.R to read the combined
column ("downloaded / total") so the chunk count checks work again.
Remove the extra blank line in NEWS.md.
|
please click Resolve so I can see which previous comments you have already addressed |
|
can you please post a screenshot like this one #272 (comment) but created with the new code? |
|
Sir @tdhock Here's the updated screenshot with the new download status table format:
Changes implemented:
All values update correctly as data downloads, and common chunk is properly counted in both downloaded and total counts. Please give your feedback and suggest what changes I need to make so that the failing test can be passed on the GitHub , Thank You ! |
|
#272 (comment) seems to have the wrong old screenshot, please revise |




Fixes #249
Summary
Adds file size information (total_MB, mean_MB, and rows) to the download status table, helping users understand disk usage for each geom.
Problem
Currently, the download status table shows how many TSV files need to be downloaded, but doesn't show file sizes. Users can't see if they're downloading 1MB or 100MB.
Test output:

Only added the test case in this first commit in the next commit will try to fix the issue !