plotly / dash-cytoscape

Interactive network visualization in Python and Dash, powered by Cytoscape.js
https://dash.plot.ly/cytoscape
MIT License
597 stars 120 forks source link

ImportError: cannot import name 'utils' #89

Closed IvoLeist closed 4 years ago

IvoLeist commented 4 years ago

Description

When pip installing from master and importing it an ImportError arises

Steps/Code to Reproduce

pip install git+https://github.com/plotly/dash-cytoscape.git@master

import dash_cytoscape as cyto

Expected Results

No error

Actual Results

import dash_cytoscape as cyto File "/venv/lib/python3.6/site-packages/dash_cytoscape/init.py", line 12, in from . import utils ImportError: cannot import name 'utils'

Versions

Python 3.6.8 (default, Apr 2 2020, 13:34:55) [GCC 4.8.5 20150623 (Red Hat 4.8.5-39)] on linux Dash 1.9.1 Dash Core Components 1.0.2 Dash HTML Components 1.8.1 ! Cytoscape errors out ! import dash_renderer; print("Dash Renderer", dash_renderer.__version) AttributeError: module 'dash_renderer' has no attribute '__version'

In order to resolve the issue:

https://github.com/IvoLeist/dash-cytoscape/commit/aaed204afb88332bdd9528cddbc4abc58f98f4da#diff-2eeaed663bd0d25b7e608891384b7298

@xhlulu Should I do a short PR or do you want to quickly fix that line?

alexcjohnson commented 4 years ago

Oh interesting - thanks @IvoLeist! I wonder if we should change it instead to:

packages=find_packages(include=[package_name, package_name + ".*"])

@xhlulu I also notice the demos folder (which would be included as a package if we used exclude=["tests*"] like we do in the base dash repo) - do general users want these installed? Obviously not under a package called "demos" but in principle they could be moved under the main package...

cc @Marc-Andre-Rivet as this class of issue may apply to all of our component packages (and the boilerplate)

xhluca commented 4 years ago

I don't think demos should be included, since it doesn't not serve any purpose for the end user (if someone wants to try out a demo, they can just clone this repo).

xhluca commented 4 years ago

what about editing the MANIFEST.in file? Instead of

include dash_cytoscape/utils

we would write:

include dash_cytoscape/utils/*

Locally, this seems to work, and we don't need to change the setup.py file. Whenever you have time, could you try that locally in a venv/conda env?

alexcjohnson commented 4 years ago

I think setup.py is a cleaner place to handle this. MANIFEST.in is typically used for non-Python files, whereas packages in the setup script packages arg is intended to cover the necessary Python files. I don't know if there are any other consequences if we were to use MANIFEST.in but I'd rather we stick to the standard.

xhluca commented 4 years ago

That makes sense; thanks for sharing those links! In this case, I think packages=find_packages(include=[package_name, package_name + ".*"]) is a good idea, this way we don't need to manually add all the subdirectories of dash_cytoscape in the future.

IvoLeist commented 4 years ago

@xhlulu just wanted to confirm your suggestion worked for me in a venv/conda env as well :)

xhluca commented 4 years ago

Awesome! Feel free to open a PR if this is time sensitive; otherwise, I'll add it to the milestone for next release (which should be soon considering the recent updates).