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

Importing pvfactors causes ShapelyDeprecationWarnings for shapely >= 1.8 #126

Closed
hashstat opened this issue Nov 1, 2021 · 9 comments · Fixed by #130
Closed

Importing pvfactors causes ShapelyDeprecationWarnings for shapely >= 1.8 #126

hashstat opened this issue Nov 1, 2021 · 9 comments · Fixed by #130

Comments

@hashstat
Copy link

hashstat commented Nov 1, 2021

When using pvfactors with shapely 1.8, ShapelyDeperecationWarnings are issued from shapely when pvfactors assigns custom attributes to shapely geometry objects.

Step to reproduce

  1. Install pvfactors 1.5.1 and shapely 1.8.0
    pip install pvfactors==1.5.1 shapely==1.8.0
  2. Import pvfactors.geometry
    python -c "import pvfactors.geometry"

Observed results
The following warnings are printed to the console:

$ python -c 'import pvfactors.geometry'
lib/python3.9/site-packages/pvfactors/geometry/base.py:349: ShapelyDeprecationWarning: Setting custom attributes on geometry objects is deprecated, and will raise an AttributeError in Shapely 2.0
  self.list_surfaces = list_surfaces
lib/python3.9/site-packages/pvfactors/geometry/base.py:350: ShapelyDeprecationWarning: Setting custom attributes on geometry objects is deprecated, and will raise an AttributeError in Shapely 2.0
  self.shaded = self._get_shading(shaded)
lib/python3.9/site-packages/pvfactors/geometry/base.py:351: ShapelyDeprecationWarning: Setting custom attributes on geometry objects is deprecated, and will raise an AttributeError in Shapely 2.0
  self.is_collinear = is_collinear(list_surfaces)
lib/python3.9/site-packages/pvfactors/geometry/base.py:352: ShapelyDeprecationWarning: Setting custom attributes on geometry objects is deprecated, and will raise an AttributeError in Shapely 2.0
  self.param_names = param_names
lib/python3.9/site-packages/pvfactors/geometry/base.py:937: ShapelyDeprecationWarning: Setting custom attributes on geometry objects is deprecated, and will raise an AttributeError in Shapely 2.0
  self.list_segments = tuple(list_segments)
lib/python3.9/site-packages/pvfactors/geometry/base.py:938: ShapelyDeprecationWarning: Setting custom attributes on geometry objects is deprecated, and will raise an AttributeError in Shapely 2.0
  self._all_surfaces = None

Expected results
The geometry module imports successfully without warnings.

Environment
Operating system: Arch Linux (up-to-date)
Python: 3.9.7 (system python using pyenv-virtualenv)
Packages: pvfactors 1.5.1, shapely 1.8.0

@hashstat
Copy link
Author

hashstat commented Nov 1, 2021

From the shapely changelog:

1.8a1 (2021-03-03)
...
Deprecations:

The following functions and geometry attributes and methods will be removed in version 2.0.0.

  • ops.cascaded_union
  • geometry .empty()
  • geometry .ctypes and .array_interface
  • multi-part geometry .len
  • setting custom attributes on geometry objects

Geometry objects will become immutable in version 2.0.0.

@anomam
Copy link
Contributor

anomam commented Nov 20, 2021

Thank you for using pvfactors and for posting this issue @hashstat .

Ideally I would like to get rid of shapely as a dependency since it's not even needed for the calculations... I started working on it here #119 and had a good start but I just haven't had time to finish it...

In the meantime, I would appreciate any help to fix this deprecation warning; it doesn't look like that would be a lot of work. Please feel free to open a PR @hashstat if you have time.
Is this on your radar @campanelli-sunpower ?

@kandersolar
Copy link
Contributor

@anomam would you rather see a "proper" fix that will allow compatibility with shapely==2.0.0, or would suppressing the warning and restricting to shapely<2.0.0 be acceptable? The latter is easy (no code reworking needed) but a poor fix in the long term. If shapely will be removed altogether at some point before too long then maybe there's no harm in a short term band-aid.

@anomam
Copy link
Contributor

anomam commented Dec 6, 2021

hello @kanderso-nrel , thanks a lot for following up on this. I am totally fine with a short term band-aid since in the long-term shapely will be removed.
I'm sorry #119 is taking me some time; I think I'll address #108 first during the holiday break, it will make my life easier I believe to rebuild the plotting part of the package and integrate it correctly with the rest.

@kandersolar
Copy link
Contributor

Ok! Well I think restricting shapely to <=1.7.1 would work, as these deprecation warnings were added in the release candidates for shapely 1.8. 1.7.1 is not so old (released Aug 20, 2020) so I think this is a reasonable approach. This would also prevent any issues if shapely 2.0 comes out before #119 gets wrapped up.

I suppose another option would be to use warnings.filterwarnings (in __init__.py?) to suppress the warnings globally. Might be a bit fiddly but it would let us keep using the latest shapely<2.0.

Any preferences here? I think I lean towards restricting to shapely<=1.7.1.

@anomam
Copy link
Contributor

anomam commented Dec 12, 2021

shapely<=1.7.1 sounds good to me! Any thoughts @campanelli-sunpower ?

@campanelli-sunpower
Copy link
Contributor

I don't see why the warnings are to be avoided, as they don't seem to break anything. I do think it is important to not (eventually) pull >=v2, because that will break things.

@kandersolar
Copy link
Contributor

I suppose the point of avoiding warnings is to not annoy the user with console clutter, which I'll certainly concede is less important than preventing breakage. But it is apparently annoying enough to prompt @hashstat to open the issue, anyway :)

I suppose if nothing else, people can find this issue and see that downgrading shapely makes the warnings disappear.

@campanelli-sunpower
Copy link
Contributor

I do apologize for my late input here. If it weren't explicitly a deprecation warning, then I'd better appreciate the alarm.

I have had issues running 1.6.4.post2 on my Mac, and am probably going to move to 1.8.0 soon in other SunPower projects, which is why I'd rather not limit the consumer here. Not sure how soon I can help remove Shapely altogether in pvfactors, but that would be a nice simplification for this library :).

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.

4 participants