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

DEP: Alias hanning as an explanatory function of hann #8148

Merged
merged 2 commits into from
Feb 10, 2018

Conversation

endolith
Copy link
Member

So there aren't duplicate help pages misleading people into thinking
that they're subtly different.

As in #228 (comment)

@endolith
Copy link
Member Author

I don't understand the build errors.

Exception occurred:
File "/home/ubuntu/scipy/build/testenv/lib/python2.7/site-packages/scipy/signal/init.py", line 345, in deco
raise RuntimeError('dev error: badly formatted doc')
RuntimeError: dev error: badly formatted doc

:/

@pv
Copy link
Member

pv commented Nov 12, 2017

Looking into the last file in the traceback probably clarifies the issue.

@rgommers rgommers added maintenance Items related to regular maintenance tasks needs-work Items that are pending response from the author scipy.signal labels Dec 9, 2017

This is for backwards compatibility. See `hann` for more details.
"""
hann(*args, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

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

Missing return statement. This line should be

return hann(*args, **kwargs)

Copy link
Member Author

Choose a reason for hiding this comment

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

derp. I'll fix that.

@WarrenWeckesser
Copy link
Member

From the discussion in #228, it looks like hanning needs a complete docstring. It was suggested there that some sort of decorator for aliases be created, but in the meantime, the docstring for hanning could be copied from hann, with an additional note added at the top stating that it is an alias for hann.

@endolith
Copy link
Member Author

the docstring for hanning could be copied from hann, with an additional note added at the top stating that it is an alias for hann.

Ok I'll do that for now.

@endolith
Copy link
Member Author

(Or this could just wait until #8319)

@endolith
Copy link
Member Author

I changed it to use numpy.deprecate instead:

@np.deprecate(new_name='hann')
def hanning(*args, **kwargs):
    return hann(*args, **kwargs)

Now it's being blocked by deco again.

@larsoner it looks like you create the deco function in #7900, do you have an opinion on it blocking these aliases? It's probably a good idea to make PRs fail if function docstrings don't have Parameters sections, but in the case of aliases I think it's ok.

@pv
Copy link
Member

pv commented Jan 28, 2018

You can just remove hanning from deprecated_windows since np.deprecated already adds the deprecation warning. The new_name however likely should be the full name, scipy.signal.windows.hann.

So there aren't duplicate help pages misleading people into thinking
that they're subtly different.
Using np.deprecate instead
@pv pv merged commit bc5c659 into scipy:master Feb 10, 2018
@pv pv removed the needs-work Items that are pending response from the author label Feb 10, 2018
@pv pv added this to the 1.1.0 milestone Feb 10, 2018
@endolith endolith deleted the hann_alias branch February 10, 2018 16:44
@endolith endolith changed the title Alias hanning as an explanatory function of hann DEP: Alias hanning as an explanatory function of hann May 28, 2022
r9y9 added a commit to nnsvs/nnsvs that referenced this pull request Jul 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks scipy.signal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants