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

refactoring into submodules #103

Merged
merged 7 commits into from
Mar 4, 2020
Merged

refactoring into submodules #103

merged 7 commits into from
Mar 4, 2020

Conversation

choldgraf
Copy link
Contributor

@choldgraf choldgraf commented Feb 29, 2020

This refactors jupyter_sphinx into a few submodules:

  • ast.py
  • execute.py
  • thebelab.py
  • utils.py

and puts the sphinx setup() function in __init__.py.

the docs build and the tests pass for me. I still want to clean up the imports, and apply a black pass, but wanted to see if this looks reasonable to others first. If folks are +1 on this, then I'll add another commit that cleans up the import statements, and applies "black" and ensures a flake8 test passes.

The one user-facing change is that the extension will now be loaded with jupyter_sphinx instead of jupyter_sphinx.execute. Perhaps there's a way we could import one into the other to temporarily keep both behaviors?

@akhmerov
Copy link
Member

Wouldn't an execute module that's a complete copy of __init__ (perhaps with a deprecation warning) not solve it?

Otherwise backwards compatibility breaking would also be OK, but then I'd like to go sure that also the directives and the AST are stabilized before making a release.

@choldgraf
Copy link
Contributor Author

ah good call - I just added a commit that is a little bit hacky, but I think accomplishes the same thing. Also added a deprecation warning. WDYT?

I'm not sure how to test both the old and new way...

@choldgraf
Copy link
Contributor Author

choldgraf commented Mar 3, 2020

I just cleaned up the imports a bit (so they are more well-ordered) and ran black on the codebase. I also added a simple test for the old extension activation string jupyter_sphinx.execute to make sure it still works. @akhmerov what do you think?

jupyter_sphinx/__init__.py Outdated Show resolved Hide resolved
jupyter_sphinx/__init__.py Outdated Show resolved Hide resolved
jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
jupyter_sphinx/ast.py Outdated Show resolved Hide resolved
@akhmerov
Copy link
Member

akhmerov commented Mar 4, 2020

Thanks a lot!

@akhmerov akhmerov merged commit 3597f9a into jupyter:master Mar 4, 2020
@choldgraf
Copy link
Contributor Author

woo - OK I'll try and upstream some more stuff this week

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