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.

Sign up for GitHub

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

Jump to bottom

[DRAFT] Pyfive Masterpiece #189

Draft
wants to merge 79 commits into
base: main
Choose a base branch
from
Draft

[DRAFT] Pyfive Masterpiece #189

wants to merge 79 commits into from

Conversation

valeriupredoi
Copy link
Collaborator

@bnlawrence I opened a PR with it for ease of checking the tests 🍺

Bryan Lawrence and others added 30 commits February 29, 2024 09:00
A few wrinkles to sort out, some changes to pyfive needed, but this i…
e9f9ee6 
…s a pre-train interim commit
bnl_test works with pyfive. The tests are borked because they're ridd…
eb9e853 
…led with zarr dependencies
Gzip decompression working. Will document issues remaining in #188
0d7bf55
Split compression and filters
d73fcd8
Slight doc improvement in hdf2numcodec
0c4272f
Need to turn off decoding filter pipeline if there is none.
c20b259
Support for contiguous data. Passing lots of tests now.
68e49d0
Remove the file context manager for now
b745d5e
A couple more h5py lurkers and one more bug squashed.
74f8662
@valeriupredoi
removed redundant test file
b628a68
@valeriupredoi
removed redundant test cases
1803e7f
@valeriupredoi
making sure we know Pyfive is run haha
f5a375c
@valeriupredoi
fix bigger data test with native pyfive exception
d1c9282
@valeriupredoi
fix package test
69c3dca
@valeriupredoi
mark lock test as xfail
c50c3d2
toy for testing missing test
09b4b3f
Merge branches 'pyfive' and 'pyfive' of github.com:valeriupredoi/PyAc…
03254dc 
…tiveStorage into pyfive
@valeriupredoi
turn off some printing for now
8a7e35b
@valeriupredoi
starting to understand the test flakiness
cacb94e
Addresses issues with missing data and with thread safety (basically …
ce0479c 
…we need to read the chunk cache before sending things off into the weeds).
Merge branch 'pyfive' of github.com:valeriupredoi/PyActiveStorage int…
39884bc 
…o pyfive
Fixed the regression wrt files with no chunking introduced by thread …
02d6120 
…safety.
Made some changes to Active just so we can use masked data as a stand…
2ffd644 
…alone method which I don't like. We would be better to pull that out and do this differently. I also fixed the disgusting test ...
@valeriupredoi
fix test compression
408558f
@valeriupredoi
fix the other compression test
3294052
@valeriupredoi
fix mocker tests
ca30bb2
@valeriupredoi
minor fix to active
c1b2823
@valeriupredoi
ass test todo
3c1b420
@valeriupredoi
fix last test
589c772
@valeriupredoi
checkout and install pyfive in dev mode
c022040
@valeriupredoi valeriupredoi added the enhancement New feature or request label Mar 8, 2024
@valeriupredoi valeriupredoi requested a review from bnlawrence March 8, 2024 15:10
@codecov Codecov
Copy link

codecov bot commented Mar 8, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 94.61078% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 90.54%. Comparing base (fe79079) to head (4816902).
Report is 19 commits behind head on main.

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

Files Patch % Lines
activestorage/storage.py 83.33% 3 Missing ⚠️
activestorage/active.py 98.34% 2 Missing ⚠️
activestorage/hdf2numcodec.py 90.90% 2 Missing ⚠️
activestorage/reductionist.py 60.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #189      +/-   ##
==========================================
+ Coverage   88.40%   90.54%   +2.14%     
==========================================
  Files           8        7       -1     
  Lines         664      550     -114     
==========================================
- Hits          587      498      -89     
+ Misses         77       52      -25

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Bryan Lawrence and others added 21 commits March 11, 2024 13:20
Adding the ability for Active to optionally self report metrics
08c8b87
Merge branch 'pyfive' of github.com:valeriupredoi/PyActiveStorage int…
2475b18 
…o pyfive
Adding some experimental data
20acfdd
Plotting and data code.
54967d2
Added more organised metrics to Active, available after a getitem fro…
20b36d1 
…m .metric_data attribute
Active _version1 and _version2 differ again
ddcff5c
Active v1 on S3 now works. Stopping default verbosity from reductionist
df6b5bb
test runner
ef001ee
@davidhassell
all-zero chunks
7c1e704
@davidhassell
new test file
209d3fa
@valeriupredoi
fix previously failing Anon bkt test
620b403
@valeriupredoi
timeOut for v2, v1 does the pancakes now
b49ffa0
@valeriupredoi
timeOut for v2, v1 does the pancakes now
e99a509
@valeriupredoi
and again v2
d1dc658
@valeriupredoi
another pancake
b43dd21
@valeriupredoi
Merge pull request #196 from valeriupredoi/zero-chunk
4816902 
Fix failure for chunks with all-zero values
@davidhassell
zero count
b5d5925
@valeriupredoi
fix test
8b46b4b
@valeriupredoi
Merge pull request #197 from valeriupredoi/zero-chunk
e4f2067 
Set count to 0 for 0-sized data
@valeriupredoi
Merge branch 'main' into pyfive
648684a
@valeriupredoi
add a conda list cmd
2152ac5
@valeriupredoi valeriupredoi mentioned this pull request Jul 22, 2024
Merged
@valeriupredoi
Copy link
Collaborator Author

all tests back to green @bnlawrence 🍺 @davidhassell pls update your branch for testing, if you hit issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnlawrence bnlawrence Awaiting requested review from bnlawrence

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@valeriupredoi @davidhassell