Skip to content

Conversation

theengineear
Copy link
Contributor

A recent Plotly error unearthed an import * statement that made interpreting the root cause difficult. In an attempt to have that never be an issue again, this PR removes all the import * statements.

As a side note, the import * statements were acting on modules with a well-defined __all__ objects. However, that's not possible to tell from a Traceback, you still just see import *.

__all__: https://docs.python.org/2/tutorial/modules.html#importing-from-a-package

Before __all__ was used to manage these. But it's clearer to
just import what we want users to see.
We *want* users to be able to do:

```python
from plotly.graph_objs import *
```

NO ONE should do:

```python
from plotly.graph_objs.graph_objs import *
```

The former is a nicely-controlled namespace. The latter is not.
It'd be nice to not let folks `import *` plotly.plotly, but that's
a little overkill and perhaps actually useful for fiddling.
"XBins",
"YAxis",
"YBins"]
__all__ = None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Notable: This will force an error when someone tries to import * this module. This is handy for us in particular since we're allowing users to import * the containing package. I find the error message somewhat useful (i.e. not un-useful).

>>> from plotly.graph_objs.graph_objs import *
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'NoneType' object does not support indexing
>>> from plotly.graph_objs import *  # this is still OK

I believe this is the functionality we want.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@theengineear
Copy link
Contributor Author

@bpostlethwaite , @chriddyp , taking out all the import * stuff. Cool?

@theengineear
Copy link
Contributor Author

thanks @etpinard

theengineear added a commit that referenced this pull request Sep 18, 2014
@theengineear theengineear merged commit 3056f20 into master Sep 18, 2014
@theengineear theengineear deleted the andrew-remove-import-all branch September 18, 2014 18:00
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.

2 participants