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

update the pad utility function and make it part of the public API #478

Merged
merged 5 commits into from
Oct 5, 2019

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Mar 17, 2019

This PR moves pad from _doc_utils.py to _dwt.py and adds it to the top-level API (as pywt.pad). I chose _dwt.py because the DWT-based functions are the ones that currently use this type of boundary handling, but it could just as well go into _utils.py or elsewhere.

The function was updated to have n-dimensional support and handle pad_widths in a manner consistent with NumPy .

The handling of pad_widths is based on inspection of numpy.lib.arraypad._as_pairs. Because it is not part of the public numpy API, I thought it was best to just replicate the behavior locally.

closes #473

correct boundary mode name: zeros->zero
DOC: enable intersphinx extension to allow linking to numpy docs

DOC: consistently link Modes documentation from docstrings
pywt/_dwt.py Outdated
def pad(x, pad_widths, mode):
"""Extend a 1D signal using a given boundary mode.

This function operates like ``numpy.pad`` but supports all signal extension
Copy link
Member

Choose a reason for hiding this comment

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

given that you added intersphinx, I think you wanted this to be single backticks to create a link?

Copy link
Contributor Author

@grlee77 grlee77 Oct 4, 2019

Choose a reason for hiding this comment

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

That just gave me numpy.pad in italics. To get the intersphinx link to work I have to use :func:`numpy.pad`. Is there a configuration option that removes the need for explicitly adding :func:? What do you prefer?

Copy link
Member

Choose a reason for hiding this comment

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

I know for SciPy it works, see for example sweep_poly linking to numpy.poly1d in its Notes section.

In the SciPy conf.py I don't immediately see what makes it work though, and that conf.py isn't exactly maintainable. So happy to use :func: here, simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the SciPy conf.py I don't immediately see what makes it work though

in a recent discussion at scikit-image, @NelleV indicated that it is the presence of
default_role = "autolink" in SciPy's conf.py
(We have not set a default_role in PyWavelets conf.py)

pywt/_dwt.py Outdated Show resolved Hide resolved
pywt/_dwt.py Outdated Show resolved Hide resolved
pywt/_dwt.py Show resolved Hide resolved
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

LGTM, just some minor comments

@rgommers rgommers merged commit 399196a into PyWavelets:master Oct 5, 2019
@rgommers
Copy link
Member

rgommers commented Oct 5, 2019

Merged, thanks @grlee77!

@grlee77 grlee77 deleted the pad branch November 13, 2019 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add an equivalent of Matlab's wextend?
2 participants