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

Use load_ipython_extension to register doit magic functions #192

Closed
wants to merge 4 commits into from

Conversation

tonyfast
Copy link
Contributor

@tonyfast tonyfast commented Sep 4, 2017

With this update

%load_ext doit.tools

replaces

from doit.tools import register_doit_as_IPython_magic
register_doit_as_IPython_magic()

This works using load_ipython_extension instead of register_doit_as_IPython_magic .

Here is an example notebook with this change.

Update the source and documentation for the IPython tool extension.
The update simplifies loading by using the load_ipython_extension
namespace instead of register_doit_as_IPython_magic.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.73% when pulling fb1517f on tonyfast:load_ipython_extension into 0c518d6 on pydoit:master.

@schettino72
Copy link
Member

@ankostis , since you wrote the original code can you help review this?

Thanks @tonyfast , it looks better but a few questions:

  • is this load_ipython_extension a new thing? is there any advantage in using the register style?
  • can we have %load_ext doit instead of %load_ext doit.tools
  • it would be nice to maintain compatibility for a while (add a deprecation warning), or just support both styles forever.
  • please add an entry on CHANGES and AUTHORS

@tonyfast
Copy link
Contributor Author

tonyfast commented Sep 5, 2017

@schettino72
Copy link
Member

have you noticed your changes broke the build?

load_ipython_extension is a function so it is True
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0001%) to 99.73% when pulling 6d49d44 on tonyfast:load_ipython_extension into 0c518d6 on pydoit:master.

@tonyfast
Copy link
Contributor Author

Missed that. I sorted the pyflakes error and now everything passes.

@schettino72
Copy link
Member

thanks. I have finally (squashed) and merged this. I tested it and it works fine :)

Seems github is not smart enough to detect I merged manually.

gabc pushed a commit to M32Media/doit that referenced this pull request Dec 5, 2017
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