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

Fix interactive mode detection #2503

Merged
merged 2 commits into from Dec 2, 2013
Merged

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Oct 7, 2013

@jrevans: Does fix the bulk of your issue in this comment?

9fbd29f#commitcomment-4267815

I'm not sure I agree with your assertion that is_interactive() === rcParams['interactive'], but I do think we should keep python -i script.py working.

@efiring
Copy link
Member

efiring commented Oct 7, 2013

@mdboom, I did not even know that sys.flags existed. Evidently sys.flags.interactive is true if the "-i" option is used when starting Python. From the man page, I don't have any idea what the -i option really does, and how it relates to mpl. So, the is_interactive() function is getting ever more mysterious. This is not a good trend. I'm not sure what the solution is, though.

@mdboom
Copy link
Member Author

mdboom commented Oct 7, 2013

The -i option runs a script and then drops into the interactive interpreter. The problem with the earlier test alone is that sys.ps1 doesn't get defined until the interactive prompt starts (so not while the script is being run).

I agree these are fairly obscure parts of Python, but they are documented, thus I'm not too worried about using them.

@@ -1191,7 +1191,8 @@ def interactive(b):

def is_interactive():
'Return true if plot mode is interactive'
b = rcParams['interactive'] and hasattr(sys, 'ps1')
b = rcParams['interactive'] and (
hasattr(sys, 'ps1') or sys.flags.interactive)
Copy link
Member

Choose a reason for hiding this comment

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

For the benefit of future readers of the code, it might be worth adding a comment, something like:

# ps1 exists if the python interpreter is running in an interactive console;
# sys.flags.interactive is true if a script is being run via "python -i".

@jrevans
Copy link

jrevans commented Oct 7, 2013

Although it makes the example I gave work, it really doesn’t fix anything.

You are working with the assumption that ‘interactive’ means a user is sitting right there at your definition of a console. You are circumventing the functionality the user expects to see when explicitly enabling matplotlib interactivity, where they expect to see plots update when turned on. Your fix is only for a bug that is really a matter of a user forgetting they turned on interactivity when they are expecting non-interactive behavior. This blocks users who expect interactivity because they explicitly turn it on.

The ‘sys.flag.interactive’ flag is a read-only flag set by the Python executable. This is not the only way in which an interactive python shell is to be had.

The end issue is, if I as a user explicitly enable matplotlib interactive behavior, then that is what I expect to get, not some other unknown behavior that makes assumptions based on how I might or might not have gotten a Python environment.

Perhaps a compromise would be to set the rcParam to be a tri-state value (0/False, 1/True, 2?). If the user explicitly calls ‘ion’, then it is set to the third state so that the check can say ‘oh, the user is telling me to be interactive’ and thus have the expected behavior.

Thanks,

--James Evans

From: Michael Droettboom [mailto:notifications@github.com]
Sent: Monday, October 07, 2013 11:19 AM
To: matplotlib/matplotlib
Cc: James Evans
Subject: [matplotlib] Fix interactive mode detection (#2503)

@jrevans https://github.com/jrevans : Does fix the bulk of your issue in this comment?

9fbd29f#commitcomment-4267815 9fbd29f#commitcomment-4267815

I'm not sure I agree with your assertion that is_interactive() === rcParams['interactive'], but I do think we should keep python -i script.py working.


You can merge this Pull Request by running

git pull https://github.com/mdboom/matplotlib fix-interactive

Or view, comment on, or merge it at:

#2503

Commit Summary

  • Consider ourselves interactive if sys.flags.interactive is set.

File Changes

Patch Links:

@efiring
Copy link
Member

efiring commented Oct 8, 2013

On 2013/10/07 1:21 PM, James Evans wrote:

Your fix is only for a bug that is really a matter of a user forgetting
they turned on interactivity when they are expecting non-interactive
behavior. This blocks users who expect interactivity because they
explicitly turn it on.

I'm lost. Would you explain, please, what are the circumstances under
which Mike's version of is_interactive would block interactive behavior
when it would otherwise be possible?

The tri-state option makes no sense to me whatsoever; presumably I am
missing something.

Would it help if ion() triggered a warning or exception under the
circumstances where is_interactive returns False? Or if there were at
least a warning under any circumstance when rcParams['interactive'] is
set in an inherently non-interactive environment? (I think this would
amount to moving the new is_interactive checking functionality into the
rcParams validator.)

Eric

@mdboom
Copy link
Member Author

mdboom commented Oct 8, 2013

You are working with the assumption that ‘interactive’ means a user is sitting right there at your definition of a console.

Yes. Exactly.

The ‘sys.flag.interactive’ flag is a read-only flag set by the Python executable. This is not the only way in which an interactive python shell is to be had.

Yes. The other is when sys.ps1 is set. That's the case in the regular Python console, IPython, and Spyder. If there is another case that needs to be covered, we can add it. But this is the documented way to test for when an interactive REPL loop is running.

Is there a case (without this change) where turning ion() on and running a script actually works? When I put the change in, I tested all GUI backends, and none of them worked -- it would just result in either no window being displayed or a window briefly being displayed. So I don't see how removing this change enables anything. If you can help me reproduce a case where that works, I'm happy to reconsider this.

@jrevans
Copy link

jrevans commented Oct 8, 2013

The specific problem that I am seeing is due to the fact that we have applications with either an embedded python shell, as well as a command-line launcher that is much like the Python command-line launcher, but with extensive error handling, logging, event-loop handling, etc. We do all of this using the standard Python C interface in exactly how it should be done. We also support the ‘-i’ flag, but cannot set the ‘sys.flags’ because that is only used by the Python executable and unfortunately not the only way to get to the python shell. Either way, there are other times in which the python interpreter might be used “interactively” without having ‘sys.ps1’ set or ‘sys.flags.interactive’ set.

In such cases a user should have the option to explicitly turn on matplotlib interactivity.

After thinking on this some more, I think I was originally not fully understanding the problem you were originally trying to solve. What it seem to me is the problem you are having is when a user might have interactive turned on in their rcparams, but then run a non-interactive script and the ‘show’ command will pop-up the windows and then close them too fast to be any good. And your fix is to prevent them from shooting themselves in the foot.

Typically I am okay with preventing users from shooting themselves in the foot, just not at the expense of removing functionality. In those cases we have always gone with “If it hurts when you do that, then don’t do that”. This is where the ‘tri-state’ setting comes in. This allows the informed user to say, ‘I want interactive behavior’ and face the consequences if they are really not interactive. You allow the uninformed user to blissfully go on their way using the shell non-interactively even when they set a global interactive flag that they forgot about, while allowing those who call ‘ion’ to actually get interactive behavior. I will be happy to code it up and submit the pull request if you want.

--James

From: Michael Droettboom [mailto:notifications@github.com]
Sent: Monday, October 07, 2013 8:03 PM
To: matplotlib/matplotlib
Cc: James Evans
Subject: Re: [matplotlib] Fix interactive mode detection (#2503)

You are working with the assumption that ‘interactive’ means a user is sitting right there at your definition of a console.

Yes. Exactly.

The ‘sys.flag.interactive’ flag is a read-only flag set by the Python executable. This is not the only way in which an interactive python shell is to be had.

Yes. The other is when sys.ps1 is set. That's the case in the regular Python console, IPython, and Spyder. If there is another case that needs to be covered, we can add it. But this is the documented way to test for when an interactive REPL loop is running.

Is there a case (without this change) where turning ion() on and running a script actually works? When I put the change in, I tested all GUI backends, and none of them worked -- it would just result in either no window being displayed or a window briefly being displayed. So I don't see how removing this change enables anything. If you can help me reproduce a case where that works, I'm happy to reconsider this.


Reply to this email directly or view it on GitHub #2503 (comment) .Image removed by sender.

@efiring
Copy link
Member

efiring commented Oct 8, 2013

On 2013/10/08 6:27 AM, James Evans wrote:

Typically I am okay with preventing users from shooting themselves in
the foot, just not at the expense of removing functionality. In those
cases we have always gone with “If it hurts when you do that, then don’t
do that”. This is where the ‘tri-state’ setting comes in. This allows
the informed user to say, ‘I want interactive behavior’ and face the
consequences if they are really not interactive. You allow the
uninformed user to blissfully go on their way using the shell
non-interactively even when they set a global interactive flag that they
forgot about, while allowing those who call ‘ion’ to actually get
interactive behavior. I will be happy to code it up and submit the pull
request if you want.

This is analogous to the Axes adjustable kwarg options:

*adjustable*       [ 'box' | 'datalim' | 'box-forced']

Using named values like this is preferable to using numeric codes. I
suspect for 'interactive' we could handle True, False, and
'True-forced', or something like that.

@efiring
Copy link
Member

efiring commented Nov 30, 2013

@mdboom, If I understand correctly, this can be merged, and then @jrevans can submit a PR for the tri-state modification. Make sense? I hate seeing things like this just sit in the stack with no decisive action.

@mdboom
Copy link
Member Author

mdboom commented Dec 2, 2013

Yes, let's merge this.

mdboom added a commit that referenced this pull request Dec 2, 2013
Fix interactive mode detection
@mdboom mdboom merged commit 0b98a14 into matplotlib:master Dec 2, 2013
@mdboom mdboom deleted the fix-interactive branch August 7, 2014 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants