Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
(feat):
read_elem_as_dask
method #1469(feat):
read_elem_as_dask
method #1469Changes from 13 commits
d111f04
00be7f0
fd635d7
f5e7fda
ae5396c
664336a
2370215
52002b6
5ab1ad1
aa1006e
dda7d83
fd418f0
23b0bfd
97b8031
1fc4cc3
5ca71ea
3672c18
33c3599
157e710
375000d
241904a
42d0d22
0bba2c0
0d0b43a
762d4c6
c935fe0
23e0ea2
5b96c77
0907a4e
20ced16
36ae8f2
bb6607e
419691b
a23df34
fd2376a
ae723d0
42b1093
78de057
717b997
2b86293
8d5a9df
449fc1a
1522de3
71c150d
b441366
0307a1d
ee075cd
89acec4
48b7630
8712582
cda8aa7
e8f62f4
f6e48ac
4de3246
ba817e0
438d28d
296ea3f
cf13a57
d02ba49
6970a97
2282351
810cd0a
a996081
f6e457b
a0b4057
4416526
fbe44f0
8c1f01d
a7d412a
4d56396
2012ee5
f65f065
8f6ea49
155a21e
daae3e5
c415ae4
00010b8
0bd87fc
5997678
f208332
eb69fcb
477bbef
103cad6
8d23f6f
9b647c2
d6d01bc
5ef93e1
9cfe908
99fc6db
b5bccc3
e5ea2b0
6ac72d6
989dc65
36b0207
dadfb4d
ca6cf66
eabaf35
4c398c3
d6fc8a4
33aebb2
701cd85
43b21a2
0e22449
ec546f4
7c2e4da
1ba5b99
49c0d49
3666735
3a332ad
d0f4d13
6e89e14
ea29cfa
f4ff236
f50b286
712e085
24dd18b
79d3fdc
9a2be00
d3bcddf
84fdc96
2608bc3
e551e18
13e3bb1
2935e45
bf0be15
9d37fc8
1ffe43e
f9df5bc
a85da39
3bef77c
899184f
efb70ec
118f43c
f742a0a
ae68731
f5e7760
96b13a3
9c68e36
410aeda
31a30c4
80fe8cb
b314248
02d4735
6e5534a
d26cfe8
94e43a3
5160016
43da9a3
9735ced
f1730c3
9861b56
235096a
dce9f07
2725ef2
ffe89f0
6cb231e
75a64fc
3f734fe
c4c2356
cc67a9b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please read the pathlib docs. This should almost definitely be
PurePosixPath
. I really hope we don’t use filesystem paths for other abstract paths anywhere, that would really mess things up.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should be a
PurePosixPath
? We don't control theelem
, so I think you need to cast it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
elem.path
is the relative path inside of the zarr hierarchy, right? converting that toPath
(a concrete path) makes no sense. It’s probably not too bad, but as said: Please read the intro here: https://docs.python.org/3/library/pathlib.htmlThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@flying-sheep I was just going to ask - why a
PurePosixPath
instead ofPurePath
? I guess since it is relative?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the first line of the description here:
pathlib.PurePath
and here: https://github.com/zarr-developers/zarr-python/blob/ace96f569490882e89c181387d27f20b0babd6a1/src/zarr/v2/util.py#L344-L345Check warning on line 96 in src/anndata/_io/specs/lazy_methods.py
Codecov / codecov/patch
src/anndata/_io/specs/lazy_methods.py#L96
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're going to use these in more places, I would suggest not making them square. This has definitely caused problems for us in the past. Maybe even make the shape/ chunk size a parameter?
I would also write a short doc string about what is actually being returned. I think it's a little strange that one of these groups has two things in it, while other only has one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just do the chunking along the axis. The shape shouldn't matter. We can reoslve it from
sparse_format