Repository navigation
DEP: signal.lombscargle: deprecate argument precenter - #23476
Conversation
github-actions
Bot
added
scipy.signal
deprecated
|
I've added the |
|
@neutrinoceros, since the changes to lombscargle last year started some discussion with correction: there is one uncommitted |
…0. Slightly tweak deprecation message.
|
I'm a little confused as to the purpose of the functions in |
They are for supporting other array types. E.g., if you pass in a CuPy array (and have The CI failure is unrelated. I don't think you need to do anything to handle this deprecation in |
DietBru
left a comment
DietBru
left a comment
There was a problem hiding this comment.
I have just some suggestions for the docstr. Otherwise, it looks good to me.
|
Have we posted on the forum about this deprecation? |
lucascolley
added
the
needs-release-note
it seems not. Just to put this (back) on your radars as the deadline for 1.17.0 is approaching @adammj @DietBru , would be good to have a simple post like https://discuss.scientific-python.org/t/dep-stats-ttest-ind-deprecate-random-state-permutation-require-keywords-for-most-args/1552, then we can merge this if no complaints. |
|
Thanks for the reminder, @lucascolley. It's been posted: https://discuss.scientific-python.org/t/dep-signal-lombscargle-deprecate-argument-precenter/2178 |
tylerjereddy
left a comment
tylerjereddy
left a comment
There was a problem hiding this comment.
Let's give this a try. The CI is all green, and astropy folks didn't really object in the cross-linked PR (they mentioned working around the deprecation). The diff seems sensible to me, and the rendered docs for lombscargle seem decent to me with the changes here.
It is true that the forum post has no responses, but that's not unusual for a deprecation query. We could wait a few more days, but this has the appropriate approvals and has been open for a few months now too, so I think I'd prefer to get it in and reduce our list of milestoned PRs by one more. If someone really objects to the deprecation we may also be able to catch that in the RC phase and revert, but seems unlikey.
Thanks all, I'll probably squash merge, commit history a bit messy here.
tylerjereddy
removed
the
needs-release-note
First stab at deprecating
precenterargument, which was made useless after #21277 was merged last year.Reference issue
None.
What does this implement/fix?
Flags
precenterfor deprecation, as it is legacy and no longer necessary—and less useful than thefloating_meanargument (as already detailed in the Notes).Additional information
Would need to be committed after gh-23475.