Skip to content

Conversation

@f-brinkmann
Copy link
Member

@f-brinkmann f-brinkmann commented Jun 1, 2025

Changes proposed in this pull request:

Note the following: I think the tests are failing due to an error in scipy scipy/scipy#23101

  • Use scipy's new accoc_legendre instead of lpmn which will be deprecated in scipy 1.17
  • Restrict scipy to version >= 1.15.0 is done in Deprecate scipy Spherical Harmonics function #142
  • Add tests for spharpy.special.legendre_function to make sure changes work as intended
  • Minor corrections in the docstrings

To Do

  • Wait for bugfix in scipy 1.17 and update pyproject.toml accordingly, or fix bug internally as suggested in the issue linked above.

@f-brinkmann f-brinkmann added this to the v1.0.0 milestone Jun 1, 2025
@f-brinkmann f-brinkmann moved this from Backlog to Require review in Weekly Planning Jun 1, 2025
@f-brinkmann f-brinkmann mentioned this pull request Jun 3, 2025
22 tasks
@f-brinkmann f-brinkmann changed the title Deprecate scipy Legender function Deprecate scipy Legendre function Jun 13, 2025
Copy link
Member

@ahms5 ahms5 left a comment

Choose a reason for hiding this comment

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

Thank you for taking care. the failing test is really annoying^^
I'm note sure about every detail, otherwise seems to be fine

The argument as an array
cs_phase : bool, optional
Whether to use include the Condon-Shotley phase term (-1)^m or not
Whether to include the Condon-Shotley phase term (-1)^m or not
Copy link
Member

Choose a reason for hiding this comment

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

🍻

@github-project-automation github-project-automation bot moved this from Require review to Reviewer Approved in Weekly Planning Jul 4, 2025
@ahms5 ahms5 moved this from Reviewer Approved to Require review in Weekly Planning Jul 31, 2025
Copy link
Member

@ahms5 ahms5 left a comment

Choose a reason for hiding this comment

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

If it requires a certain scipy version, we should add it to the pyproject.toml

@f-brinkmann
Copy link
Member Author

If it requires a certain scipy version, we should add it to the pyproject.toml

It scheduled for scipy 1.17, but there hasn't been much action on the issue. I added a todo to the top so we don't forget.

@mberz
Copy link
Member

mberz commented Sep 16, 2025

I'm not expecting that this will be solved in time for the next release of spharpy.
I think we either have to fix this ourselves or live with the bug.

@mberz
Copy link
Member

mberz commented Sep 16, 2025

There's an update on the respective scipy issue, let's wait for further updates here scipy/scipy#23101

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Require review

Development

Successfully merging this pull request may close these issues.

3 participants