-
-
Notifications
You must be signed in to change notification settings - Fork 328
fix:*-like creation routines take kwargs #2992
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
base: main
Are you sure you want to change the base?
Conversation
requesting review from @jhamman since I think you touched this code most recently |
@@ -109,10 +109,10 @@ def _get_shape_chunks(a: ArrayLike | Any) -> tuple[ChunkCoords | None, ChunkCoor | |||
return shape, chunks | |||
|
|||
|
|||
def _like_args(a: ArrayLike, kwargs: dict[str, Any]) -> dict[str, Any]: | |||
def _like_args(a: ArrayLike) -> dict[str, object]: |
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.
Why the change to object
? And if you're feeling eager, this could maybe be a typed dict?
class LikeArgs(TypedDict):
shape: tuple[int, ...]
chunks: tuple[int, ...]
dtype: np.typing.DTypeLike
order: ...
but that's probably out of scope for this PR.
like_kwargs = _like_args(a) | kwargs | ||
if isinstance(a, (AsyncArray | Array)): | ||
like_kwargs.setdefault("fill_value", a.metadata.fill_value) |
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.
To make sure I understand the precedence, it's
- User keyword arguments (via
| kwargs
) _like_kwargs(a)
fill_value
viaa.metadata.fill_value
? It'd be worth confirming we have each of those cases covered with tests.
like_kwargs = _like_args(a) | kwargs | ||
if isinstance(a, (AsyncArray | Array)): | ||
like_kwargs.setdefault("fill_value", a.metadata.fill_value) | ||
return await empty(**like_kwargs) # type: ignore[arg-type] |
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.
Why the type ignore here? I guess the LikeArgs
typed dict might solve that.
LikeFuncName = Literal["zeros_like", "ones_like", "empty_like", "full_like", "open_like"] | ||
|
||
|
||
@pytest.mark.parametrize("func_name", get_args(LikeFuncName)) |
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.
Maybe there's a reason to structure it like this, but my usual preference is to use functions instead of names. So the actual params would be [zarr.api.asynchronous.zeros_like, zarr.api.asynchronous.one_like, ...
].
In main, array creation routines that take an existing array, like
full_like
,zeros_like
, etc, don't allow users to override certain array properties (like shape and dtype) via keyword arguments. E.g.,full_like(arr, shape=(10,))
will ignore theshape
keyword argument and produce an array with the same shape asarr
.This PR makes these array creation routines sensitive to keyword arguments like
shape
,dtype
,chunks
, etc.closes #2989