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

Module exports functions with naming conflict with submodule. #415

Closed
Carreau opened this issue Sep 17, 2016 · 2 comments
Closed

Module exports functions with naming conflict with submodule. #415

Carreau opened this issue Sep 17, 2016 · 2 comments
Milestone

Comments

@Carreau
Copy link
Member

Carreau commented Sep 17, 2016

in short nbconvert.exporters.export is both a module and a function leading to some confusing situation (for @michaelpacer) where import get confused when using relative import and some things make no sens:

In [10]: from nbconvert.exporters.export import ExporterNameError

In [11]: import nbconvert.exporters.export as e; e
Out[11]: <function nbconvert.exporters.export.export>

Wait did I just import a class from a function , and is the function it's own module ?

Proposed fix:

Rename the module nbconvert.exporters.export to something else. Deprecate and shim it (not to break import)

That's not an hard issue to fix, but it's an interesting one. I'm fine doing it, but we can leave it for someone who is interested in it and weird python import behavior.

@mpacer
Copy link
Member

mpacer commented Sep 17, 2016

I think in my case in #414, it wasn't this issue because I was using
relative imports which should only work on modules/packages and not
functions as the argument after from (e.g., from .export).

I think fixing the name ambiguity is probably a good idea, but I think I
was running into a cyclic dependency issue which was solved by moving the
import inside of a function rather than at the top level (like the codebase
was already doing with from .export import get_exporter a few lines
before my changes). I can see reason to get rid of this as well, but I
think fixing this kind of cyclic dependency in a non code-duplicating way
is going to be a to require isolating some of the exporter declaration
functionality from the exporter getting(and erring) functionality in
separate modules.

On Friday, September 16, 2016, Matthias Bussonnier notifications@github.com
wrote:

in short nbconvert.exporters.export is both a module and a function
leading to some confusing situation (for @michaelpacer
https://github.com/michaelpacer) where import get confused when using
relative import and some things make no sens:

In [10]: from nbconvert.exporters.export import ExporterNameError

In [11]: import nbconvert.exporters.export as e; e
Out[11]:

Wait did I just import a class from a function , and is the function it's
own module ?

Proposed fix:

Rename the module nbconvert.exporters.export to something else. Deprecate
and shim it (not to break import)

That's not an hard issue to fix, but it's an interesting one. I'm fine
doing it, but we can leave it for someone who is interested in it and weird
python import behavior.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#415, or mute the thread
https://github.com/notifications/unsubscribe-auth/ACXg6EUIhD1XLPtf87rbLEs26l2h69Wlks5qq00egaJpZM4J_ezz
.

Carreau added a commit to Carreau/nbconvert that referenced this issue Sep 17, 2016
nbconvert.exporters.export could be resolved either as a function or a
midule. `ExporterNameError` would also trigger some circular imports, so
moved it one level higher.

Should Fix jupyter#415
Carreau added a commit to Carreau/nbconvert that referenced this issue Sep 19, 2016
nbconvert.exporters.export could be resolved either as a function or a
midule. `ExporterNameError` would also trigger some circular imports, so
moved it one level higher.

Should Fix jupyter#415

# Conflicts:
#	nbconvert/exporters/export.py
@mpacer
Copy link
Member

mpacer commented Sep 27, 2016

#423 closes this.

@mpacer mpacer closed this as completed Sep 27, 2016
@Carreau Carreau added this to the 5.0 milestone Oct 3, 2016
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 a pull request may close this issue.

2 participants