Skip to content
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

ENH: average to allow better use w/ custom objects #12067

Closed
wants to merge 1 commit into from

Conversation

numpynewb
Copy link

@numpynewb numpynewb commented Oct 1, 2018

A simple syntax change to enable better use of average with custom Python objects.

This works because if weights is None, then avg.dtype can be "O" when working with custom Python objects.

The fix does nothing but change how np.shape is called, so that it will work with objects that don't necessarily have shapes, like base Python floats, custom Python objects, etc.

A simple syntax change to enable better use of `average` with custom Python objects. 

This works because if `weights is None`, then `avg.dtype` can be `"O"` when working with custom Python objects.

The fix does nothing but change how shape is called, so that it will work with objects that don't necessarily have shapes, like base Python floats, etc.
@numpynewb numpynewb changed the title average to allow better use w/ custom objects ENH: average to allow better use w/ custom objects Oct 1, 2018
Copy link
Contributor

@tylerjereddy tylerjereddy left a comment

Choose a reason for hiding this comment

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

If this is useful, my first thought is that it would be helpful to have a unit test that demonstrates something that works with this change that didn't work before. That would also provide a sort of "code example" for the behavior you're trying to enable.

@tylerjereddy
Copy link
Contributor

The macos Azure failure is probably not related -- we may also have to skip / decorate the test in numpy.f2py.tests.test_semicolon_split.TestCallstatement.test_callstatementas I think I saw that stochastically fail once before as well.

@numpynewb
Copy link
Author

numpynewb commented Oct 2, 2018

@tylerjereddy thank you for your quick response, and apologies for the delay. This is actually not as simple as I had initially thought when submitting the change, because, for example, a.mean(axis) can generate a base-Python float, which will also return an error in some instances where a dtype is needed. Bear with me while I put together a coherent vignette (I am somewhat new to contributing, so apologies for what I now understand to be an incomplete commit/merge request!).

I believe that this is important (selfishly, of course) because it will facilitate the use of average, and other NumPy routines that rely on it, with custom numeric-like Python objects, where arbitrary arithmetic and NumPy operations make sense.

Thanks again; I very much appreciate you responding.

@charris charris added the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label Oct 10, 2018
@charris
Copy link
Member

charris commented Oct 10, 2018

Needs release note.

Base automatically changed from master to main March 4, 2021 02:04
@seberg
Copy link
Member

seberg commented Jun 15, 2022

Going to close the PR, I expect its still relevant. But right now this is an incomplete one-line change with tests missing and was idle for many years.

If anyone wants to pick it up, np.mean for example does work-around the same issues pretty successfully, so there should be something to copy. I think gh-8696 tracks the issue also.

@seberg seberg closed this Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
01 - Enhancement 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes component: numpy.lib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants