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

allow toggle of svg/png inline figure format #398

Closed
wants to merge 1 commit into from

Conversation

minrk
Copy link
Member

@minrk minrk commented Apr 22, 2011

png is now the default, and select_figure_format(fmt) is added to user_ns for switching between the two.

This should address some of the discussion in gh-381

The default format is a configurable, but since the qtconsole doesn't actually use any config, it's not hooked up to anything.

png is now the default, and select_figure_format(fmt) is added to user_ns for switching between the two.

This should address some of the discussion in ipythongh-381
@ellisonbg
Copy link
Member

I would like to rethink how we are adding top-level functions to the user_ns. I propose we do the following:

from IPython import api
api.display(o)

Etc. We don't have to call it api, but something like this that encapsulates all of the non-magic-function stuff we are adding to the namespace. I also think we want to think of a more full featured APi in the display system that makes it easy for users to enable/disable different display formats for different objects. That is generic functionality that should be developed for any type and display format.

@minrk
Copy link
Member Author

minrk commented May 9, 2011

So 'api' is a Namespace where our injected functions go? Does that include get_ipython()? If it doesn't, why not attach these methods to the InteractiveShell instance?

shell = get_ipython()
shell.get_figs = get_figs
shell.display = display_stuff
# etc.

@ellisonbg
Copy link
Member

Not sure if we want this, but it is one possibility. Maybe api is a bad name as it will pull us back into the land of the chaos that was ipapi, where we had 2 different apis that recursively called each other. I don't want that. To clarify, I am thinking of functions that users will want to call in their regular usage of ipython interactively. I am not putting method of InteractiveShell in this category. Basically things that we either have to 1) put into builtins or 2) have the users import by hand. I think we want to keep builtins clean, so then it is a question of how exactly to have the manual import look. I don't think attaching them to InteractiveShell makes sense because InteractiveShell is to low level for most users and has a ton of other things that will confuse most users.

@minrk
Copy link
Member Author

minrk commented May 9, 2011

Okay, that's fair. But we already have an API presented as get_ipython(). If we aren't injecting into user_ns, I see exactly 3 options:

  1. implement this under the name get_ipython()
  2. have two APIs - get_ipython() and this new one
  3. add a new API and get rid of get_ipython()

What if get_ipython() returns this Namespace, and the InteractiveShell is only accessible as get_ipython().shell instead of the top-level object?

@ellisonbg
Copy link
Member

But this is confusing two things that I my mind and completely different. get_ipython has one use: to return the InteractiveShell instance. In my mind InteractiveShell is a developer focused API that 99.9% of users should never need. We don't want the average user to have to know about it or "open the hood" to get things done. The other functions we are talking about are high level user focused functions that we expect many users to user on a regular basis. These are in the same category as magic functions, but they are not "magic". Magics are injected, which pollutes the name main space at some level, BUT at least users can turn that off to clear their namespace of potential collisions.

@minrk
Copy link
Member Author

minrk commented May 9, 2011

If the average user shouldn't know about get_ipython(), then it definitely should not be injected into the user_ns or __builtins__ - that's just asking for trouble.

My suggestion would keep the function, and specifically abstract the shell away one level - get_ipython() is for accessing IPython things, and one of those things is the InteractiveShell instance. Does it not make sense that if we have a high-level and low-level API, the low-level API is retrieved by simply diving deeper into the high level one?

@ellisonbg
Copy link
Member

I agree that it probably makes sense to remove get_ipython from the namespace. This was an improvement over the previous approach of injecting names like __IPYTHON always. But there is a bit of an issue with removing get_ipython. It is is moved to a separate import, it will be difficult for that method to know which InteractiveShell to return if we move to allowing more than one instance. Of course, for now, we can just return InteractiveShell.instance().

Also, in my mind, the ipython developer API is InteractiveShell, so I am not sure what else get_ipython would return other than that. I think this is an abstraction we want, that is, to create the develop API in an object oriented manner using InteractiveShell and its children.

@minrk
Copy link
Member Author

minrk commented May 11, 2011

Okay, so from IPython import api for injected methods (or other name if we find something preferable), and eliminate get_ipython altogether in favor of InteractiveShell.instance()?

We will have to be more careful injecting things into IPython.api than user_ns if we want to be forward-thinking about InteractiveShell not being a singleton. If InteractiveShell is no longer a singleton, then IPython.api must be bound somehow to an InteractiveShell instance, and it must be allowed to have more than one IPython.api.

Do we want pngs in Qt to be part of 0.11, or 0.12? If it's 0.12, then should we merge this anyway, since people want it now?

@ellisonbg
Copy link
Member

I am still hesitant to use the name "api" because of the mass confusion it caused previously (ipapi). I would like to find a name to makes it clear that 1) the core should never, under any circumstance call functions in this module 2) it is a user focused api, not a developer/config/core focused one. With this in mind, a function like select_figure_format should not appear there.

So how to handle select_figure_format. I think (once the config dust settles) we should make the figure format an attribute of a Configurable. Then I think we want to create a top level magic called %config that takes the command line syntax that can be used to change any config variable at runtime.

As far as what to do with get_ipython and InteractiveShell.instance, I am not sure. I think if we want to eventually get rid of the singleton nature of InteractiveShell, then continuing to inject get_ipython into the user_ns is probably the best way to go. Otherwise we will have to add complexity to track which InteractiveShell should be returned.

@ellisonbg
Copy link
Member

One other point: let's get the config stuff merged. Then making these changes will be much easier.

@ellisonbg
Copy link
Member

We are closing this as this work is continuing here:

#451

@ellisonbg ellisonbg closed this May 17, 2011
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

2 participants