-
-
Notifications
You must be signed in to change notification settings - Fork 361
Update stateful/property tests. #3161
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
Conversation
Add actions to 1. overwrite data with oindex 2. read and compare a full array
6883a0e
to
6fa55f0
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3161 +/- ##
==========================================
- Coverage 94.76% 94.76% -0.01%
==========================================
Files 78 78
Lines 8672 8710 +38
==========================================
+ Hits 8218 8254 +36
- Misses 454 456 +2
🚀 New features to boost your workflow:
|
Nice not sure which PR ended up fixing this! |
can you explain a bit more what's changing in this PR? |
self.all_arrays.add(path) | ||
|
||
@rule() | ||
@with_frequency(0.25) |
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.
from logs it seems like hypothesis was clearing the store quite frequently, so I'm reducing frequency here.
store=store, | ||
fill_value=fill_value, | ||
zarr_format=3, | ||
dimension_names=data.draw( |
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.
fixes an oversight where we were never setting dimension names on these arrays
|
||
@precondition(lambda self: bool(self.all_arrays)) | ||
@rule(data=st.data()) | ||
def overwrite_array_orthogonal_indexing(self, data: DataObject) -> None: |
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.
Adds a new step modeling a user overwriting existing array data with .oindex
) | ||
|
||
|
||
def v3_dtypes() -> st.SearchStrategy[np.dtype[Any]]: |
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.
These are unnecessary now. I guess we could add DeprecationWarning asking the user to use zarr.testing.strategies.dtypes()
instead. I can do this next week
name = draw(array_names) | ||
attributes = draw(attrs) | ||
zarr_format = draw(zarr_formats) | ||
store = draw(stores, label="store") |
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.
just nicer logs.
npindexer = [] | ||
ndim = len(shape) | ||
for axis, size in enumerate(shape): | ||
val = draw( |
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.
strategy now works for 0-D
st.none() | shard_shapes(shape=nparray.shape, chunk_shape=chunk_shape), | ||
label="shard shape", | ||
) | ||
extra_kwargs["dimension_names"] = draw( |
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.
again, i forgot to set dimension_names earlier
|
||
|
||
@pytest.mark.filterwarnings("ignore::zarr.core.dtype.common.UnstableSpecificationWarning") | ||
@given(data=st.data(), zarr_format=zarr_formats) |
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.
Not needed now that all dtypes are supported for both versions
|
||
@precondition(lambda self: bool(self.all_arrays)) | ||
@rule(data=st.data()) | ||
def check_array(self, data: DataObject) -> None: |
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.
new check that asserts the model and the tested store have the same array data
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.
thanks deepak!
Add actions to
[Description of PR]
TODO:
docs/user-guide/*.rst
changes/