MAINT: Hide decorator from pytest traceback#28211
Conversation
Currently it points the user to the internal numpy function
```
kwargs = {'strict': False}, old_name = 'y', new_name = 'desired'
@functools.wraps(fun)
def wrapper(*args, **kwargs):
for old_name, new_name in zip(old_names, new_names):
if old_name in kwargs:
if dep_version:
end_version = dep_version.split('.')
end_version[1] = str(int(end_version[1]) + 2)
end_version = '.'.join(end_version)
msg = (f"Use of keyword argument `{old_name}` is "
f"deprecated and replaced by `{new_name}`. "
f"Support for `{old_name}` will be removed "
f"in NumPy {end_version}.")
warnings.warn(msg, DeprecationWarning, stacklevel=2)
if new_name in kwargs:
msg = (f"{fun.__name__}() got multiple values for "
f"argument now known as `{new_name}`")
raise TypeError(msg)
kwargs[new_name] = kwargs.pop(old_name)
> return fun(*args, **kwargs)
E AssertionError:
E Arrays are not equal
E
E (shapes (2, 2, 200, 200), (2, 2, 800, 800) mismatch)
E ACTUAL: array([[[[0, 0, 0, ..., 0, 0, 0],
E [0, 0, 0, ..., 0, 0, 0],
E [0, 0, 0, ..., 0, 0, 0],...
E DESIRED: array([[[[0, 0, 0, ..., 0, 0, 0],
E [0, 0, 0, ..., 0, 0, 0],
E [0, 0, 0, ..., 0, 0, 0],...
../../miniforge3/envs/dev/lib/python3.10/site-packages/numpy/_utils/__init__.py:85: AssertionError
```
|
Seems like an improvement to me. CI failure is unrelated. @mdhaber since you did a lot of work recently on these assertion helpers, what do you think about this? |
|
IIUC, it's unfortunate that the use of
It's still probably not a bad idea to hide the decorator traceback from the decorator, though. (Unless the problem actually occurs in the decorator, which would be unusual.) |
|
Ah good call @hmaarrfk did you check how this error looks right now on |
Great feel free to close this. I guess it stumped me |
|
It won't happen on main. good enough for me. fyi, you might want to remove the import of and maybe go as far as getting rid of the utility function alltogether? I once wrote these deprecators with tests: https://pypi.org/project/wabisabi/ I know numpy is all about being lightweight, but without the usage of |
|
Take this as a little user story. i'm still on numpy 1.26 since some libraries (tensorflow) only just got to 2.X. So you might want to leave in some deprecations a little longer than you think you should. |
This was discussed in the linked issue. I proposed removing it according to the standard policy; I left it to other maintainers to decide whether an exception was needed. There is always going to be a mismatch between how often developers and users would like updates to happen, so there has to be a compromise somewhere. Users that don't update often enough will miss important warnings and updates. Consider updating in smaller increments if necessary. (Every other version / once per year should be enough to catch all deprecation warnings.) I did forget to remove the import. Typically linting would catch that sort of thing, but I guess that rule is not enabled. I'd suggest the rule be enabled and the import be removed in that PR. I don't think the function needs to be removed. It will inevitably be useful later. |
|
i'm back here 2 months later wishing that an update to numpy would have resolved this issue. I generally think that this fix is "in line" with the best practices of numpy. I see little harm in pressing "merge" Any chance to merge this in? |
|
Oops, I see what happened. gh-27738 (which fixes this by removing the decorator from these functions entirely) merged to Since the PR was closed, we didn't merge and backport it with 2.2.3. Let's merge it now and mark it for backport in 2.2.4 (if there is one). The decorator is not applied to these functions in 2.3, so it will definitely be fixed there.
So incidentally, the deprecation was left in place longer than usual! |
|
Thanks! I think since the plan was to keep the decorators alive it will help in the next round anyway!!! |
Currently it points the user to the internal numpy function
Whit this PR: