Remove %guiref and add instead an entry in our Help menu to show its contents #98

ccordoba12 · 2016-02-14T17:45:14Z

Fixes #78

I'll create a corresponding pull request in the IPython repo to remove gui_reference from it.

takluyver · 2016-02-14T19:03:31Z

This PR is still registering %guiref as a magic. I was initially thinking of a more dramatic change, where we don't involve the kernel in showing it at all, and just make an item in the help menu that brings it up. I know some of the text refers specifically to IPython, though - do you think it's feasible to rewrite these bits so the references to IPython are more examples than prescription? Or is it too tightly wedded to IPython to make any sense for other kernels?

ccordoba12 · 2016-02-14T19:17:47Z

This PR is still registering %guiref as a magic

Yes, but only for ipykernel. As far as I know, ipykernel is the only kernel that shows %guiref as part of its banner. i tried to see if I could remove that line but I didn't understand how to do it.

However, I think it's ok to define %guiref when using ipykernel because

ipykernel is able to handle magics
People already know about the existence of %guiref.

and just make an item in the help menu that brings it up

I also did that as part of this pull request.

do you think it's feasible to rewrite these bits so the references to IPython are more examples than prescription?

I removed most of the references to IPython in gui_reference, and updated some imports too (like IPython.core.display to IPython.display). Please review it to see if things can be further improved :-)

takluyver · 2016-02-15T13:19:10Z

I think I'd prefer to remove the %guiref magic entirely:

It's only applicable to one frontend, so we shouldn't be showing it in the general banner when starting an IPython kernel. The banner should also show up in the terminal frontend (though it doesn't right now: kernel banner missing jupyter/jupyter_console#59), and it's supposed to be independent of the frontend.
It only works if the Qt console is installed in the same Python that the kernel is running in, which is by no means guaranteed.
It's one more special case we can get rid of for our own kernel.

The banner that mentions %guiref is defined in IPython.core.usage and used in ipykernel.zmqshell. I'm happy to clear up both of those modules if you want to remove the magic registration in this PR.

ccordoba12 · 2016-02-15T14:50:56Z

It only works if the Qt console is installed in the same Python that the kernel is running in

You're totally right. It makes no sense to maintain %guiref because it's not entirely dependent on the kernel.

So please remove its references from ipykernel and IPython.core.usage, then I'll remove it from here.

Note: The thing that confused me is that several things about %guiref are still present in qtconsole, so I assumed you removed it in a rebase or something. This time I'll make sure to remove all supporting code about it :-)

We're getting rid of the %guiref magic - see spyder-ide/qtconsole#98

takluyver · 2016-02-15T15:03:20Z

Removed in ipython/ipykernel#101 and ipython/ipython#9231

ccordoba12 · 2016-02-15T15:50:31Z

Ok, I think this is ready, unless you have some comments about my changes to gui_reference.

ccordoba12 · 2016-02-15T15:56:47Z

%guiref also appears in this figure, so it should be updated accordingly. Maybe @willingc could help us with that?

takluyver · 2016-02-15T16:29:27Z

qtconsole/usage.py

+
+This console is designed to emulate the look, feel and workflow of a terminal
+environment, while adding a number of enhancements that are simply not possible
+in a real terminal, such as inline syntax highlighting, true multiline editing,


We might want to change the phrasing, because the first two of these (syntax highlighting and multiline editing) are coming to IPython for version 5 (see ipython/ipython#9118). And even inline graphics are possible, albeit not supported by most terminal emulators.

The console is designed to emulate the look, feel and workflow of a terminal environment. Beyond this basic design, the console may also implement functionality not currently found in most terminal emulators. Some examples of these console enhancements are inline syntax highlighting, multiline editing, inline graphics, and others.

takluyver · 2016-02-15T16:48:03Z

I know most of the things I'm pointing out in the text are not things you changed, but that text is long overdue for an update, and this seems like a good opportunity to go through it. Bad luck!

@willingc , could you also do a pass on this to help make it clearer? Thanks.

willingc · 2016-02-15T18:45:06Z

qtconsole/jupyter_widget.py


    def _started_channels(self):
-        """Reimplemented to make a history request and load %guiref."""
+        """Reimplemented to make a history request"""


Perhaps a semantic quibble on my part but "reimplemented" feels not quite accurate to me. BaseFrontendWidget is abstract; FrontEndWidget implements the method from the abstract BaseFrontEndWidget; JupyterWidget subclasses from FrontEndWidget.

The docstrings for each method in this section are not consistent. "Override method" or "Redefine method" feel more accurate to me. Now that I'm thinking about it, it may just be best to add a comment here:

#--------------------------------------------------------------------------- # 'BaseFrontendMixin' abstract interface # # For JupyterWidget, override FrontendWidget methods which implement the # BaseFrontend Mixin abstract interface #---------------------------------------------------------------------------

willingc · 2016-02-15T18:56:09Z

qtconsole/usage.py

+terminal IPython: single expressions are immediately evaluated, and indented
+blocks are evaluated once a single blank line is entered::
+
+    In [1]: print "Hello Jupyter!"  # Enter was pressed at the end of the line


print("Hello Jupyter!")

willingc · 2016-02-15T19:25:21Z

@ccordoba12 @takluyver I've gone through and added my comments. Depending on the time window for 4.2 release, it might be good to merge after these changes are made. After that I would be happy to iterate any further changes as needed.

@takluyver

- This follows @takluyver and @willingc review.

ccordoba12 · 2016-02-21T08:24:40Z

Thanks for your review. I addressed all comments, except for one from @takluyver about the use of subprocess.

willingc · 2016-02-21T09:45:27Z

@ccordoba12 Looks great! I noticed @takluyver had a wording suggestion. Other than that, this is ready to merge.

Remove %guiref and add instead an entry in our Help menu to show its contents

ccordoba12 · 2016-02-22T20:49:55Z

@minrk or @takluyver, I think everything is ready now for you to release 4.2 :-)

willingc · 2016-02-22T20:52:22Z

@ccordoba12 🍰 🍪

minrk · 2016-02-23T08:04:42Z

@ccordoba12 released 4.2! Thanks for your help.

ccordoba12 · 2016-02-23T13:11:01Z

Thanks a lot Min!!

El 23/02/16 a las 03:04, Min RK escribió:

@ccordoba12 https://github.com/ccordoba12 released 4.2! Thanks for
your help.

—
Reply to this email directly or view it on GitHub
#98 (comment).

temok-cse · 2016-04-25T18:25:00Z

Dear All,

Wishes for the QtConsole:

Enable searches like in Emacs (control-s) or like in the less command (slash '/').
This would be useful in situations like when browsing the QtConsole Help.
Enable the display of the QtConsole Help from a command in the QtConsole (in addition or in replacement of using the mouse to click on the Help menu).
A shortcut to move to other tabs.
Cheers,
Temok

ccordoba12 · 2016-04-25T18:42:23Z

Regarding your points @temok-mx:

This is implemented in Spyder, i.e. if you press Ctrl+F you get a search dialog to easily look for something in Qtconsole content.
Previously Qtconsole had a command called %guiref to do precisely this, but it was removed by this PR. The reasoning behind this decision is described in Remove %guiref and add instead an entry in our Help menu to show its contents #98 (comment)
That's a good idea, but someone will have to implement it. You could open a new issue about it :-)

temok-cse · 2016-04-25T20:08:32Z

Thanks Carlos,

I just posted an issue for point 3): #122
Saludos,
Temok

ccordoba12 added 2 commits February 14, 2016 12:06

Fix %guiref and also move its text here from IPython

374cbc6

Add entry to Help menu to show guiref contents

38cb0c4

ccordoba12 added this to the 4.2 milestone Feb 14, 2016

ccordoba12 mentioned this pull request Feb 14, 2016

Remove gui_reference and page_guiref because they were moved to qtconsole ipython/ipython#9229

Merged

ccordoba12 changed the title ~~Fix %guiref and add an entry in our Help menu to show its contents~~ Remove %guiref and add instead an entry in our Help menu to show its contents Feb 15, 2016

takluyver added a commit to takluyver/ipykernel that referenced this pull request Feb 15, 2016

Don't show %guiref in banner

1da6720

We're getting rid of the %guiref magic - see spyder-ide/qtconsole#98

takluyver mentioned this pull request Feb 15, 2016

Don't show %guiref in banner ipython/ipykernel#101

Merged

Remove all code related to %guiref

3b41a87

takluyver reviewed Feb 15, 2016
View reviewed changes

willingc reviewed Feb 15, 2016
View reviewed changes

ccordoba12 added 3 commits February 21, 2016 02:56

Improve docstrings of overriden methods from BaseFrontendMixin

a7988fc

Remove unneeded page_guiref function

2194941

Improve gui_reference text

e69f0cd

- This follows @takluyver and @willingc review.

Simplify text about running subprocesses in qtconsole

8f9d0ca

ccordoba12 added a commit that referenced this pull request Feb 22, 2016

Merge pull request #98 from ccordoba12/fix-guiref

8d7b1f6

Remove %guiref and add instead an entry in our Help menu to show its contents

ccordoba12 merged commit 8d7b1f6 into spyder-ide:master Feb 22, 2016

ccordoba12 deleted the fix-guiref branch February 22, 2016 20:48

Remove %guiref and add instead an entry in our Help menu to show its contents #98

Remove %guiref and add instead an entry in our Help menu to show its contents #98

Uh oh!

Conversation

ccordoba12 commented Feb 14, 2016

Uh oh!

takluyver commented Feb 14, 2016

Uh oh!

ccordoba12 commented Feb 14, 2016

Uh oh!

takluyver commented Feb 15, 2016

Uh oh!

ccordoba12 commented Feb 15, 2016

Uh oh!

takluyver commented Feb 15, 2016

Uh oh!

ccordoba12 commented Feb 15, 2016

Uh oh!

ccordoba12 commented Feb 15, 2016

Uh oh!

takluyver Feb 15, 2016

Choose a reason for hiding this comment

Uh oh!

willingc Feb 15, 2016

Choose a reason for hiding this comment

Uh oh!

takluyver commented Feb 15, 2016

Uh oh!

willingc Feb 15, 2016

Choose a reason for hiding this comment

Uh oh!

willingc Feb 15, 2016

Choose a reason for hiding this comment

Uh oh!

willingc commented Feb 15, 2016

Uh oh!

ccordoba12 commented Feb 21, 2016

Uh oh!

willingc commented Feb 21, 2016

Uh oh!

ccordoba12 commented Feb 22, 2016

Uh oh!

willingc commented Feb 22, 2016

Uh oh!

minrk commented Feb 23, 2016

Uh oh!

ccordoba12 commented Feb 23, 2016

Uh oh!

temok-cse commented Apr 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccordoba12 commented Apr 25, 2016

Uh oh!

temok-cse commented Apr 25, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

temok-cse commented Apr 25, 2016 •

edited

Loading