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

Why does fill_betweenx not have interpolate? #6543

Closed
rmlopes opened this issue Jun 6, 2016 · 5 comments
Closed

Why does fill_betweenx not have interpolate? #6543

rmlopes opened this issue Jun 6, 2016 · 5 comments
Labels
API: changes Difficulty: Easy https://matplotlib.org/devdocs/devel/contribute.html#good-first-issues
Milestone

Comments

@rmlopes
Copy link
Contributor

rmlopes commented Jun 6, 2016

Hi,

I am working with seismic wigglets, and the lack of interpolate on the fill_betweenx creates very poor filling on my vertical wigglets. Why isn't there an option to interpolate as with fill_between?

Tks

@rmlopes rmlopes changed the title Why does fill_betweenx does not have interpolate? Why does fill_betweenx **not** have interpolate? Jun 6, 2016
@rmlopes rmlopes changed the title Why does fill_betweenx **not** have interpolate? Why does fill_betweenx not have interpolate? Jun 6, 2016
@tacaswell
Copy link
Member

What do you mean by 'interpolate'? It should have the same behavior as 'plot'. If you want to do a higher order interpolation than linear, do that as pre-processing before passing the data into fill_between

@rmlopes
Copy link
Contributor Author

rmlopes commented Jun 6, 2016

The interpolate is a keyword argument in fill_between.

What I mean is that if I plot the traces horizontally and use fill_between the wiglets are correctly filled, that is, the entire area under a curve is filled. But if I plot the traces vertically and then use fill_betweenx the wiglets are missing pieces close to the crossover points (in some cases it is just a line).

I deduce from the documentation that this is because there is no interpolation and the wiiglet filling is constrained to the data points available.

Shouldn't it be done in the same way even though we are swapping the axis (there is no interpolate option in fill_betweenx)?

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Jun 8, 2016
@tacaswell
Copy link
Member

Sorry, I should read the docs before commenting more of the time 🐑

That is an oversite the fill_between has it and fill_betweenx does not (although you are the first person in 5 years to notice the asymetry!).

If you look at the source of the two functions, porting the functionality should not be too hard.

The only dicey bit is we have to decide if the API on fill between will be

  def fill_betweenx(self, y, x1, x2=0, where=None, interpolate=False,
                      step=None, **kwargs):

to match the fill_between API

or

  def fill_betweenx(self, y, x1, x2=0, where=None, 
                      step=None, interpolate=False, **kwargs):

to not break the API of fill_betweenx.

I think this is a case where being python3 only and having kwarg-only parameters would be helpful.

@rmlopes Want to take a crack an implementing this?

@tacaswell tacaswell added Difficulty: Easy https://matplotlib.org/devdocs/devel/contribute.html#good-first-issues API: changes labels Jun 8, 2016
@rmlopes
Copy link
Contributor Author

rmlopes commented Jun 8, 2016

Sure, it will be my pleasure.

I think this is a case where being python3 only and having kwarg-only parameters would be helpful.

Agreed.

I guess the best would be not to break the fill_betweenx API for backwards compatibility, since anyway most of the use cases I have found specify the name of the argument when calling the function. I actually thought it was a keyword argument, not positional.

What I would suggest is just to use it as a keyword argument in fill_betweenx. If you have old code is not a problem, and if you confuse it with the fill_between API and do not use the name of the arguments it will most probably result in error. I am not sure what happens if you give a boolean by mistake in the step argument.

@rmlopes rmlopes mentioned this issue Jun 8, 2016
@QuLogic
Copy link
Member

QuLogic commented Dec 9, 2016

Fixed by above PR.

@QuLogic QuLogic closed this as completed Dec 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API: changes Difficulty: Easy https://matplotlib.org/devdocs/devel/contribute.html#good-first-issues
Projects
None yet
Development

No branches or pull requests

3 participants