Conversation
|
Thanks @rjzamora! @ian-r-rose could you take a look at this when you get a moment? |
| lock=None, | ||
| mode="a", | ||
| ): | ||
| class HDFFunctionWrapper: |
There was a problem hiding this comment.
Would love to be able to use a Protocol here. Some day...
| assert_eq(df16, out) | ||
|
|
||
| # Test getitem optimization | ||
| with tmpfile("h5") as fn: |
| if division and global_divisions: | ||
| global_divisions = global_divisions[:-1] + division | ||
| elif division: | ||
| global_divisions = division |
There was a problem hiding this comment.
Ouch, this is tricky. Can we make the divisions global to begin with?
There was a problem hiding this comment.
Since this PR is just a refactor, most of the actual logic is just a copy of what already existed (iow, this is not my code, and I haven't thought through it carefully). With that said, I can try to revist this later and simplify the logic a bit.
There was a problem hiding this comment.
Yeah, I can appreciate not wanting to mess with that logic too much
jrbourbeau
left a comment
There was a problem hiding this comment.
Apologies for the delayed response here @rjzamora. Would you mind taking care of the merge conflict here and then we can merge this in
No worries! Conflict should be resolved. I will also fix any problems if a fresh ci run turns up any failures. |
|
I'm going to merge this since it's passing CI :) |
Superceeds #7284 (since that PR is now quite stale)
Follows same approach as #7415 and #7615 to use Blockwise for Dask-Dataframe's read_hdf. A slight refactor of the original logic was required.