API: Soft deprecate signal.* windows #7900
Conversation
larsoner
added
the
backport-candidate
|
The CI failure is about the deprecated functions not being in the refguide. The way it checks for deprecation is to actually try using the function to see if a DeprecationWarning is emitted. So I think I'll either need to change the |
|
Add it to one of the skiplist's here: https://github.com/scipy/scipy/blob/master/tools/refguide_check.py#L118 |
|
I originally tried that, that list isn't used for the check I'm talking
about unfortunately. That allows things not to be in `__all__`. The problem
here is that they are in `__all__` but not in the refguide.
Should I make a similar new list and modify the checker? Or does this
indicate that we should emit a warning on use?
|
|
Oh wait there is another list below that one that might work. Let me try
that one, too.
|
|
Nope, adding them to |
|
Travis is happy (other than OSX delays) so this is ready for review/merge from my end |
|
Definitely don't want that deprecation for 1.0, and preferably not at all - way too much code is affected, for a minor benefit. I'll be happy to look at modifying the checker this weekend if needed. |
|
Could you also add |
|
I'll modify the checker today and remove the DeprecationWarning (at least it was good for finding the other cases I needed to fix!). I was just wary of doing so because it seemed to indicate a break from standard practice, given there was no standard way to deprecate in the manner suggested. But I agree it could break a lot of code for minimal benefit so probably isn't worth it. |
|
I didn't need to modify the testing framework, I was just using the skip-list RegEx incorrectly :( I've now added Ready for review/merge from my end. |
|
Mostly looks good, two comments:
|
|
@rgommers done |
scipy/signal/tests/test_windows.py
Outdated
|
|
||
|
|
||
| def test_deprecation(): | ||
| assert 'signal.hann is deprecated' in dep_hann.__doc__ |
There was a problem hiding this comment.
Odd, this test should not be passing on TravisCI with OPTIMIZE=-OO. That is run under Python 3.4, and I've checked that it picks up -OO, because it says at the top of the test output
WARNING: assertions not in test modules or plugins will be ignored because assert
statements are not executed by the underlying Python interpreter (are you using python -O?)
It does fail as expected for me on Python 2.7. I'll push a commit moving the -OO run to 2.7
There was a problem hiding this comment.
Note that there are 2 issues here, use of assert and use of __doc__
There was a problem hiding this comment.
Good points, should be fixed now (your commit broke Travis as intended)
There was a problem hiding this comment.
No it didn't break, that's the special failures with numpy master. I added -OO to the wrong build matrix entry.
There was a problem hiding this comment.
I'll remove my commit and then merge. Will figure out -OO some other time, opening a new issue for it.
There was a problem hiding this comment.
Here's the link to the TravisCI run (the link to it will disappear from this PR once I force-push): https://travis-ci.org/scipy/scipy/builds/278796885?utm_source=github_status&utm_medium=notification
|
Merged, thanks Eric. |
|
I'm tending towards not backporting this to 1.0.x. There's no urgent need for this, and we're close to rc1. |
rgommers
removed
the
backport-candidate
Ready for review/merge from my end. Would be nice to have for 1.0.0.
Closes #7898.