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 to redefine default MetaKernel magics with custom ones #172

Merged
merged 2 commits into from
Sep 20, 2018

Conversation

ccordoba12
Copy link
Contributor

We're building an ipdb kernel for Spyder with MetaKernel. For that we defined Pdb commands as magics and some of their names collide with MetaKernel default magics.

With this PR users could create magics that are loaded instead of MetaKernel ones with the same name.

@coveralls
Copy link

coveralls commented Sep 12, 2018

Coverage Status

Coverage decreased (-0.2%) to 75.028% when pulling c7b5b0e on ccordoba12:redefine-default-magics into 36fcede on Calysto:master.

@ccordoba12
Copy link
Contributor Author

@blink1073, @dsblank, any chance of getting this one reviewed? We're needing it to keep working on our IPdb kernel.

Thanks!

@dsblank
Copy link
Member

dsblank commented Sep 17, 2018

Sorry, just saw this! I'll take a look.

@ccordoba12
Copy link
Contributor Author

Thanks!

By the way, this is a fantastic project!! Thanks a lot for working on it.

@dsblank
Copy link
Member

dsblank commented Sep 17, 2018

@ccordoba12 This looks simple and fine; thanks! It would be great to document this someplace, maybe just in the doc string (I know there isn't a lot of docs). Or if not there, how about here:

https://github.com/Calysto/metakernel/blob/master/docs/source/installation.rst

@blink1073
Copy link
Contributor

I also approve pending docs, thanks!

@ccordoba12
Copy link
Contributor Author

I added a docstring to the Magic class, which is the one you look at to define your own magics, explaining how to redefine default magics in your own kernels.

Please let me know what you think about it.

@blink1073
Copy link
Contributor

LGTM, thanks Carlos!

@blink1073 blink1073 merged commit 967e803 into Calysto:master Sep 20, 2018
@blink1073
Copy link
Contributor

@dsblank, are we ready to cut 0.21?

@dsblank
Copy link
Member

dsblank commented Sep 20, 2018

@ccordoba12 Thanks! @blink1073 I think so. Thanks! I'll update the HISTORY with the other changes (and this one) soon.

@ccordoba12 ccordoba12 deleted the redefine-default-magics branch September 20, 2018 14:26
@ccordoba12
Copy link
Contributor Author

Thanks @blink1073 and @dsblank!

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.

4 participants