-
Notifications
You must be signed in to change notification settings - Fork 23
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
Add import check for shapely/geos #110
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @kanderso-nrel, this is the right approach in my opinion. shapely
is currently marked as a requirement for the whole package so we shouldn't allow partial usage of pvfactors and do a module-level warning, it would be quite messy I think.
|
||
Windows users may also be able to resolve the issue by installing wheels from `Christoph Gohlke`_. | ||
|
||
.. _Christoph Gohlke: https://www.lfd.uci.edu/~gohlke/pythonlibs/#shapely |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
"from another source like conda-forge with " | ||
"`conda install -c conda-forge shapely`, or alternatively from " | ||
"Christoph Gohlke's website if you're on Windows: " | ||
"https://www.lfd.uci.edu/~gohlke/pythonlibs/#shapely" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
Closes #109
One consequence of doing this check in
__init__.py
is that it makes it impossible to use pvfactors at all ifshapely.geos.lgeos
can't be imported. I've not actually used pvfactors before so maybe this is a silly question, but is there any pvfactors functionality that doesn't require shapely? If so, maybe this warrants a more targeted change.I ran into difficulties testing py2.7, but here's the output I get with py3.7:
I'm happy to push updates if this needs anything else like a changelog entry or something, let me know.