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

scipy 1.11 compatibility #196

Merged
merged 4 commits into from Jul 6, 2023
Merged

scipy 1.11 compatibility #196

merged 4 commits into from Jul 6, 2023

Conversation

kandersolar
Copy link
Member

@kandersolar kandersolar commented Jul 5, 2023

  • [ ] Closes #xxx
  • [ ] Added tests to cover all new or modified code.
  • [ ] Clearly documented all new API functions with PEP257 and numpydoc compliant docstrings.
  • [ ] Added new API functions to docs/api.rst.
  • [ ] Non-API functions clearly documented with docstrings or comments as necessary.
  • Adds description and name entries in the appropriate "what's new" file
    in docs/whatsnew
    for all changes. Includes link to the GitHub Issue with :issue:`num`
    or this Pull Request with :pull:`num`. Includes contributor name
    and/or GitHub username (link with :ghuser:`user`).
  • Pull request is nearly complete and ready for detailed review.
  • Maintainer: Appropriate GitHub Labels and Milestone are assigned to the Pull Request and linked Issue.

The tests are failing in #186 due to unrelated changes in scipy (see the 1.11.0 release notes):

In scipy.stats.mode, the default value of keepdims is now False, and support for non-numeric input has been removed.

In this line of code, the .mode(...).mode is now returning a scalar, so trying to index with [0] fails:

lambda shifted_period: stats.mode(shifted_period).mode[0]

This PR switches away from scipy.stats.mode, which has too inconsistent an interface across all versions to be easily used here, and instead just uses pd.Series.mode.

@kandersolar kandersolar added bug Something isn't working dependency Issues relating to dependencies or dependency management labels Jul 5, 2023
@kandersolar kandersolar added this to the v0.2.0 milestone Jul 5, 2023
@kandersolar kandersolar marked this pull request as ready for review July 5, 2023 21:35
@kperrynrel
Copy link
Collaborator

Ok, I just took the two changes you applied here @kandersolar, and added them to #186 to see if we'd be passing and it looks like it is! LGTM, thank you for jumping on this so quickly :)

Copy link
Collaborator

@kperrynrel kperrynrel left a comment

Choose a reason for hiding this comment

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

Tested on #186 and passed!

@kandersolar kandersolar merged commit b52ca9a into pvlib:main Jul 6, 2023
36 checks passed
@kandersolar kandersolar deleted the mode_bugfix branch July 6, 2023 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependency Issues relating to dependencies or dependency management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants