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: add a neigborwise function #9428

Closed
wants to merge 1 commit into from

Conversation

eric-wieser
Copy link
Member

This ends up being something I write a lot. It would be handy for matplotlib/matplotlib#6404 and axes.pcolor.

Naming and argument order perhaps needs work. Suggestions?

@bashtage
Copy link
Contributor

What about a general purpose moving window function which would apply a function to n consecutive points. In the example you show, how much slower is this than (x[1:]+x[:-1]) /2?

@eric-wieser
Copy link
Member Author

how much slower is this than (x[1:]+x[:-1]) /2?

For large arrays, the speed is the same. The overhead of building the slices should be small.

@njsmith
Copy link
Member

njsmith commented Jul 16, 2017

New public APIs need a mailing list post.

The more general/conventional way of naming this pattern is moving/sliding window, as @bashtage notes. This is something that pandas has some specialty in – might be worth checking out what they do.

@seberg
Copy link
Member

seberg commented Jul 17, 2017

Aren't a lot of these operations also much faster using convolutions?

for ax in axis:
x_begin = x[slices[:ax] + begin_sl + slices[ax+1:]]
x_end = x[slices[:ax] + end_sl + slices[ax+1:]]
x = func(x_begin, x_end)
Copy link
Member Author

@eric-wieser eric-wieser Aug 1, 2017

Choose a reason for hiding this comment

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

To be used as a backend for diff, this would need to be flipped

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. This is probably better than adding a method to ufuncs. It will also be much easier to introduce this to the ma module than a ufunc method.

Copy link
Member Author

@eric-wieser eric-wieser Aug 1, 2017

Choose a reason for hiding this comment

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

Why does the ma module need its own version? What would be different

Copy link
Contributor

Choose a reason for hiding this comment

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

It may not. I am not 100% sure that it will combine the masks correctly otherwise though.

@madphysicist
Copy link
Contributor

@seberg, @njsmith. This is too specialized to be a sliding window. This applies to non-commutative functions like subtract, which would be one of the primary uses. A convolution kernel may not even be possible for every reasonable value of func.

Copy link
Contributor

@madphysicist madphysicist left a comment

Choose a reason for hiding this comment

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

Please ignore my comments, they are more notes than anything else.


def neighborwise(x, func, axis=None):
"""
Apply a function to pairs of neigbours along the given axes
Copy link
Contributor

Choose a reason for hiding this comment

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

I am pretty sure a period is required at the end of this line.

Can be used to resample a grid at the center of each cell, useful for
matploblib's `pcolor`:

>>> midpoint = lambda a, b: (a + b) / 2
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be done with just np.add if you multiply the result by 0.5.

func : function
A function taking two arguments, to call on the (vectorized) pairs of
neighbours. The first argument is the values at the lower indices
axis : int or sequence of int
Copy link
Contributor

Choose a reason for hiding this comment

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

It was my impression that None usually means to operate on a raveled version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Technically any iterable seems to work here.

>>> midpoint = lambda a, b: (a + b) / 2
>>> x, y = np.indices((3, 4)) * 2
>>> x
array([[0, 0, 0, 0],
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be (0, 2, 4) repeated, or remove the * 2 in the line above.

[1, 1, 1, 1],
[2, 2, 2, 2]])
>>> y
array([[0, 1, 2, 3],
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be (0, 2, 4, 6) repeated.

@seberg
Copy link
Member

seberg commented Aug 3, 2017

I did not mean that this function can always be replaced by a convolution. What I meant was that I do not quite like that in many cases where the function might seem like "the right tool", it may actually not be. And if those cases are a big portion of the likely use cases, I get unsure.

Base automatically changed from master to main March 4, 2021 02:04
@charris charris added the 52 - Inactive Pending author response label Apr 6, 2022
@charris charris closed this Apr 6, 2022
@seberg
Copy link
Member

seberg commented Apr 6, 2022

@charris how about we also add a label like "Good start" or so? Just to make it a bit easier to find these to reopen them (I am not sure that "Inactive" quite covers that part).

@charris
Copy link
Member

charris commented Apr 6, 2022

Sure, "Inactive" was an existing label, so I reused it. Maybe "Good idea"? Someone might be motivated to follow up.

@seberg
Copy link
Member

seberg commented Apr 6, 2022

"Good idea" sounds good. Will add it later if you don't and then tag some of these at least (and anyone should feel free to tag more).

@seberg seberg added the 64 - Good Idea Inactive PR with a good start or idea. Consider studying it if you are working on a related issue. label Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
01 - Enhancement 52 - Inactive Pending author response 64 - Good Idea Inactive PR with a good start or idea. Consider studying it if you are working on a related issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants