Skip to content

Restrict shapely dependency to <=2#130

Merged
campanelli-sunpower merged 4 commits into
SunPower:masterfrom
kandersolar:shapely171
Jan 11, 2022
Merged

Restrict shapely dependency to <=2#130
campanelli-sunpower merged 4 commits into
SunPower:masterfrom
kandersolar:shapely171

Conversation

@kandersolar

Copy link
Copy Markdown
Contributor

Closes #126.

I wonder if we should think about releasing 1.5.2 soon? Besides these deprecation warnings and the tilt=0 issue from #125, it would be nice for the latest pvfactors version to allow pvlib==0.9.0 on installation. Someone asked about this on the pvlib google group: https://groups.google.com/g/pvlib-python/c/hNaNsM1pNSw

@kandersolar

Copy link
Copy Markdown
Contributor Author

Hi @campanelli-sunpower, just a bump, especially about cutting a new release that doesn't forbid the latest pvlib :)

@campanelli-sunpower campanelli-sunpower left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Because pvfactors is a library, I think we should allow upgrading of Shapely until (but not including) v2, at which point the warnings will become failures.

Comment thread requirements.txt Outdated
Comment thread docs/sphinx/whatsnew/v1.5.2.rst Outdated
Comment thread requirements.txt Outdated

@campanelli-sunpower campanelli-sunpower left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @kanderso-nrel .

@kandersolar

Copy link
Copy Markdown
Contributor Author

I suspect the test failures are caused by the numpy change discussed in pvlib/pvlib-python#1369

If that's true (need to verify it locally), I'd advocate for the same fix: weaken the test tolerance a bit to allow for the variation from numpy

@campanelli-sunpower

Copy link
Copy Markdown
Contributor

@kanderso-nrel Looks like some small numerical drift on the newer Pythons. I think it would be ok to widen the tolerance slightly.

Note the recommendation here to not use numpy.testing.assert_almost_equal:

It is recommended to use one of assert_allclose, assert_array_almost_equal_nulp or assert_array_max_ulp instead of this function for more consistent floating point comparisons.

@kandersolar

Copy link
Copy Markdown
Contributor Author

@campanelli-sunpower Shall I update this PR with those changes?

@campanelli-sunpower

Copy link
Copy Markdown
Contributor

@kanderso-nrel Please do so if you are ok with that. It would keep the review sequence more straightforward. Thanks.

@campanelli-sunpower campanelli-sunpower left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for updating these test methods too.

@campanelli-sunpower campanelli-sunpower changed the title Restrict shapely dependency to <=1.7.1 Restrict shapely dependency to <=2 Jan 11, 2022
@campanelli-sunpower campanelli-sunpower merged commit 91c7657 into SunPower:master Jan 11, 2022
@kandersolar kandersolar deleted the shapely171 branch January 11, 2022 23:58
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.

Importing pvfactors causes ShapelyDeprecationWarnings for shapely >= 1.8

2 participants