Skip to content

MAINT: Chain ValueError in ma.timer_comparison #17133

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

Merged
merged 1 commit into from
Aug 24, 2020

Conversation

karan-dhir
Copy link
Contributor

Changes related to #15986

Adds exception chaining to ValueError in numpy/ma/timer_comparison.py

Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks. I can't help feeling this code is completely dead and isn't even run by our tests any more...

@eric-wieser eric-wieser added the component: numpy.ma masked arrays label Aug 21, 2020
@mattip
Copy link
Member

mattip commented Aug 21, 2020

assert_array_compare does not show up in the docs, other assert_array_* variants do. I don't mind merging this, it seems harmless, but maybe we should deprecate this function.

@eric-wieser
Copy link
Member

eric-wieser commented Aug 21, 2020

assert_array_compare does not show up in the docs, other assert_array_* variants do

Are you sure you're thinking of this assert_array_comparein numpy.ma.timer_comparison.ModuleTester? This seems to be a copy that really shouldn't exist.

@charris charris changed the title MAINT: changed ValueError line 103 in file timer_comparison MAINT: Chain ValueError in ma.timer_comparison Aug 24, 2020
@charris charris merged commit b4d2295 into numpy:master Aug 24, 2020
@charris
Copy link
Member

charris commented Aug 24, 2020

Thanks @karan-dhir .

It does seem that this function is unused in numpy but is part of the tested public API. We should look into removing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants