-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
use src layout and use hatch
for packaging
#1592
Changes from 3 commits
4577ed3
a5981a3
2e823a5
b9bc08f
4c04117
3ec8164
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
node: $Format:%H$ | ||
node-date: $Format:%cI$ | ||
describe-name: $Format:%(describe:tags=true,match=*[0-9]*)$ | ||
ref-names: $Format:%D$ | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
*.py linguist-language=python | ||
*.ipynb linguist-documentation | ||
.git_archival.txt export-subst |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -459,7 +459,7 @@ def _encode_storage_transformer_metadata( | |
|
||
@classmethod | ||
def _decode_storage_transformer_metadata(cls, meta: Mapping) -> "StorageTransformer": | ||
from zarr.tests.test_storage_v3 import DummyStorageTransfomer | ||
from tests.test_storage_v3 import DummyStorageTransfomer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is a class from tests imported in user-facing functionality? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we don't know! see #1601 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I see, haha! No worries, I'll rebase this once your PR is merged! |
||
from zarr._storage.v3_storage_transformers import ShardingStorageTransformer | ||
|
||
# This might be changed to a proper registry in the future | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ | |
from zarr.attrs import Attributes | ||
from zarr.storage import KVStore, DirectoryStore | ||
from zarr._storage.v3 import KVStoreV3 | ||
from zarr.tests.util import CountingDict, CountingDictV3 | ||
from tests.util import CountingDict, CountingDictV3 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a bit of surprising change. Is it required? Is this what hatch suggests packages do? Generally, I prefer a relative imports within the tests directory. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yes, this was just a personal preference. Happy to revert it! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's revert it. Happy to discuss this change elsewhere but let's keep this pr scope as small as possible. |
||
from zarr.hierarchy import group | ||
|
||
|
||
|
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.
can you explain why we need this file? Looking at hatch-vcs, I don't see any requirement of this configuration.
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.
This file allows users to install
Zarr
from Git archives without encountering version issues. This is not ahatch-vcs
thing, instead this file should've been introduced whensetuptools-scm
was introduced in Zarr - https://setuptools-scm.readthedocs.io/en/latest/usage/#builtin-mechanisms-for-obtaining-version-numbers.hatch-vcs
usessetuptools_scm
underneath.Scientific Python also recommends adding git archival support -
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.
Makes sense. Thanks for the explanation.