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.

Sign up for GitHub

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

Jump to bottom

i.atcorr: fix plot_filter in create_iwave #3911

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

i.atcorr: fix plot_filter in create_iwave #3911

wants to merge 3 commits into from

Conversation

pesekon2
Copy link
Contributor

  • missing import of pyplot
  • figures not shown correctly
  • fix wrong axes
@pesekon2
i.atcorr: fix plot_filter in create_iwave
a72bb36 
* missing import of pyplot

* figures not shown correctly

* fix wrong axes
@pesekon2 pesekon2 added bug Something isn't working Python Related code is in Python imagery labels Jun 20, 2024
@pesekon2 pesekon2 self-assigned this Jun 20, 2024
echoix
echoix reviewed Jun 25, 2024
View reviewed changes
Copy link
Member

@echoix echoix left a comment

Choose a reason for hiding this comment

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

Just like this, this PR changes the behaviour of the function. It's not exactly the same. To know if it's safe and reasonable, I'd need to know a bit more on the context and how it is used.

Is it a function part of "public api", ie used in user code in the wild, or just inside the file?

Before these changes, if I recall correctly, calling plot like this would add/change the currently active plot. In this PR, you are explicitly creating a new one, with new axes.

Before these changes, technically it would be possible to further edit the plot, and would have to call show() when needed (when it was that that was wanted, I usually returned the figure and axes for that in old code I did). Now, the function shows the plot directly here. Since only show() is used, it is a blocking function (I think it depends on the plotting backend though). For example, a non blocking call would be show(block=False) or show(blocking=False) (writing from memory, it's been years). A blocking call prevents other event processing in other parts of the code (but the new window will be able to do the interactions if the backend supports it).

So, just like this, I can't be sure if it's a safe change or not. It's maybe not a dead simple change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working imagery module Python Related code is in Python
2 participants
@pesekon2 @echoix