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

Generate "All magics..." menu live #956

Merged
merged 5 commits into from Nov 24, 2011

Conversation

Carreau
Copy link
Member

@Carreau Carreau commented Oct 31, 2011

Hi everyone,

I'm trying to catch up with the merging of the qtconsole. For now constructing the "all magic menu" by querying the kernel about get_ipython().lsmagic() through user_expressions. I thought it might be a good idea.

It does work as long as the action is not automatically done at startup, otherwise there is no prompt.
( try adding :self.window.pop.trigger() just before self.app._exec() in qtconsole.py )
You can trigger the action by hand through the Magics > Populate All Magic Menu Menu, which create a second All Magics Menu.

I don't know why, it's not working at startup, and i'm not sure adding a timer to execute this action later is a good idea.
I was also thinking of updating at each new prompt, or every time the user click on Magic Menu.

If can get a little help on this ...

Otherwise, still not 100% back, still digging in my backups to recover some data, but will do my best.

Matthias

@Carreau
Copy link
Member Author

Carreau commented Nov 4, 2011

Hi,

Found what I was looking for. I've boud the action to update the menu to a first_response signal.
The menu is now populated with all the magics from %lsmagic.

I've also append '?' to the end of some magic which failed when invoked alone.

@Carreau
Copy link
Member Author

Carreau commented Nov 10, 2011

rebased on master, and forced push..

@fperez
Copy link
Member

fperez commented Nov 11, 2011

Glad to have you back @Carreau. I'll try to review this over the weekend, if nobody beats me to it.

@fperez
Copy link
Member

fperez commented Nov 11, 2011

I found a bit of time to do it now :)

I like this and I think it's useful, as it will encourage users to browse around and try some magics they might not know about. It also seems to work perfectly on my system. I'll add some inline comments that are fairly minor code tidying issues and should be pretty easy to address.

@@ -137,6 +138,7 @@ class FrontendWidget(HistoryConsoleWidget, BaseFrontendMixin):
self._input_splitter = self._input_splitter_class(input_mode='cell')
self._kernel_manager = None
self._request_info = {}
self._callback_dict=dict([])
Copy link
Member

Choose a reason for hiding this comment

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

Create the emtpy dict with {} like the line above, for consistency.

@fperez
Copy link
Member

fperez commented Nov 11, 2011

OK, I've left a bunch of feedback. Most of it is easy to address and mostly code formatting/documentation issues that you'll get used to quickly. There's only one issue that probably will require some code refactoring.

But overall it's a nice improvement, thanks again!

@Carreau
Copy link
Member Author

Carreau commented Nov 11, 2011

@fperez,
Just pushed a squashed commit of all the suggestion you made, I was a little lazy about code formating.
If you are ok with that I can push a squashed commit of everything rebased on the last master.

@fperez
Copy link
Member

fperez commented Nov 11, 2011

Sure, go ahead and force push a cleaned up commit if you prefer. I need to tune out of ipython for the day, I have some local work I must tend to now. But I'll get back to this as soon as I can, probably tomorrow.

@Carreau
Copy link
Member Author

Carreau commented Nov 12, 2011

Squashed, rebased on master, and forced pushed ...

@Carreau
Copy link
Member Author

Carreau commented Nov 16, 2011

Rebase on master agin to stay up to date....

# append '?' to the end to print the docstring when called from the menu
protected_magic = set(["more","less","load_ext","pycat","loadpy","save"])

for magic in eval(listofmagic):
Copy link
Member

Choose a reason for hiding this comment

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

Why are you using eval here? Eval is a fairly dangerous construct that should only be necessary in very specialized places, I don't see why it's needed here...

Copy link
Member Author

Choose a reason for hiding this comment

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

you'r right, it can be done by regExp ... that was because the return value by the kernel was the repr() of the list, so I thought to use eval without trying harder

	replace the `eval` to transform the `repr()` of the magic list send by the kernel
	by a regular expression, change the docstring to be numpy complient (I hope so)
@Carreau
Copy link
Member Author

Carreau commented Nov 24, 2011

Sooooory ... i fixed everything 2 or 3 days ago and forgot to push...
I rebased and foced push...

@fperez
Copy link
Member

fperez commented Nov 24, 2011

Great, very nice! Thanks for the excellent contribution, merging now. Everything looks good here.

fperez added a commit that referenced this pull request Nov 24, 2011
Generate "All magics..." menu in the Qt console dynamically, so it correctly shows available magics instead of an internally hardcoded list.
@fperez fperez merged commit 8fa6544 into ipython:master Nov 24, 2011
fperez added a commit that referenced this pull request Nov 28, 2011
Basically it reverts the effect of #956 and goes back to a static list
for the 'all magics' menu.  I tried to mark very clearly the new code
so we can disable it once a proper fix for #1057 is committed, but we
can't have a broken qt console in master.
Carreau added a commit to Carreau/ipython that referenced this pull request Nov 29, 2011
    This reverts commit 65546bf, done to
    temporaly fixed a race condition introduced by ipython#956, next commits should
    fixe this race condition
Carreau added a commit to Carreau/ipython that referenced this pull request Nov 29, 2011
	fix race condition in qtconsole due to a race condition beween the population
	of the magic menu and the first prompt request. Resolved by upgrading the
	logic of the console to handle several executions request in parallel.

	Some namedTuple might still carry redundent information but the fix is the
	first priority

	fixes ipython#1057 , introduced by ipython#956
fperez added a commit that referenced this pull request Nov 30, 2011
Fix race condition in qtconsole between the population of the magic menu and the first prompt request. 

Resolved by upgrading the logic of the console to handle several executions request in parallel.

Closes #1057, introduced by #956.
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Generate "All magics..." menu in the Qt console dynamically, so it correctly shows available magics instead of an internally hardcoded list.
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Basically it reverts the effect of ipython#956 and goes back to a static list
for the 'all magics' menu.  I tried to mark very clearly the new code
so we can disable it once a proper fix for ipython#1057 is committed, but we
can't have a broken qt console in master.
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
    This reverts commit 65546bf, done to
    temporaly fixed a race condition introduced by ipython#956, next commits should
    fixe this race condition
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
	fix race condition in qtconsole due to a race condition beween the population
	of the magic menu and the first prompt request. Resolved by upgrading the
	logic of the console to handle several executions request in parallel.

	Some namedTuple might still carry redundent information but the fix is the
	first priority

	fixes ipython#1057 , introduced by ipython#956
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Fix race condition in qtconsole between the population of the magic menu and the first prompt request. 

Resolved by upgrading the logic of the console to handle several executions request in parallel.

Closes ipython#1057, introduced by ipython#956.
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