Skip to content
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

progressbar for inserts/upserts of all fileformats, closes #485 #486

Merged
merged 3 commits into from Sep 15, 2022

Conversation

MischaU8
Copy link
Contributor

@MischaU8 MischaU8 commented Sep 8, 2022

@codecov
Copy link

codecov bot commented Sep 15, 2022

Codecov Report

Base: 96.47% // Head: 96.52% // Increases project coverage by +0.04% 🎉

Coverage data is based on head (0acbc68) compared to base (d9b9e07).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 0acbc68 differs from pull request most recent head d5db749. Consider uploading reports for the commit d5db749 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #486      +/-   ##
==========================================
+ Coverage   96.47%   96.52%   +0.04%     
==========================================
  Files           6        6              
  Lines        2642     2646       +4     
==========================================
+ Hits         2549     2554       +5     
+ Misses         93       92       -1     
Impacted Files Coverage Δ
sqlite_utils/cli.py 95.86% <100.00%> (ø)
sqlite_utils/utils.py 94.98% <100.00%> (+0.47%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@simonw
Copy link
Owner

simonw commented Sep 15, 2022

Annoying mypy test failure:

/Users/runner/hostedtoolcache/Python/3.10.7/x64/lib/python3.10/site-packages/numpy/__init__.pyi:636:
error: Positional-only parameters are only supported in Python 3.8 and greater

Looks like this:

@simonw
Copy link
Owner

simonw commented Sep 15, 2022

There's a fix for mypy that has landed but isn't out in a release yet:

For the moment looks like pinning to Python 3.10.6 could help. Need to figure out how to do that in GitHub Actions though.

@simonw
Copy link
Owner

simonw commented Sep 15, 2022

https://github.com/actions/setup-python/blob/main/docs/advanced-usage.md#using-the-python-version-input says can set the full version:

- uses: actions/setup-python@v4
  with:
    python-version: "3.10.6" 

I'll try that.

@simonw
Copy link
Owner

simonw commented Sep 15, 2022

This looks good to me. I need to run some manual tests before merging (it's a good sign that the automated tests pass though).

@simonw
Copy link
Owner

simonw commented Sep 15, 2022

I had a big CSV file lying around, I converted it to other formats like this:

sqlite-utils insert /tmp/t.db t /tmp/en.openfoodfacts.org.products.csv --csv
sqlite-utils rows /tmp/t.db t --nl > /tmp/big.nl
sqlite-utils rows /tmp/t.db t > /tmp/big.json

Then tested the progress bar like this:

sqlite-utils insert /tmp/t2.db t /tmp/big.nl --nl

Output:

sqlite-utils insert /tmp/t2.db t /tmp/big.nl --nl
  [------------------------------------]    0%
  [#######-----------------------------]   20%  00:00:20

With --silent it is silent.

And for regular JSON:

sqlite-utils insert /tmp/t3.db t /tmp/big.json 
  [####################################]  100%

This is actually not doing the right thing. The problem is that sqlite-utils doesn't include a streaming JSON parser, so it instead reads that entire JSON file into memory first (exhausting the progress bar to 100% instantly) and then does the rest of the work in-memory while the bar sticks at 100%.

@simonw
Copy link
Owner

simonw commented Sep 15, 2022

I'm going to land this anyway. The lack of a streaming JSON parser is a separate issue, I don't think it should block landing this improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants