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

Add alias to gaussian_filter #193

Merged
merged 3 commits into from
Mar 7, 2021
Merged

Add alias to gaussian_filter #193

merged 3 commits into from
Mar 7, 2021

Conversation

K-Monty
Copy link
Contributor

@K-Monty K-Monty commented Feb 24, 2021

Closes #192

Before you submit a pull request, check that it meets these guidelines:

  1. The pull request should include tests.
  2. If the pull request adds functionality, the docs should be updated. Put
    your new functionality into a function with a docstring, and add the
    feature to the list in README.rst.
  3. The pull request should work for Python 3.6, 3.7, and 3.8. Check
    https://travis-ci.org/dask/dask-image/pull_requests
    and make sure that the tests pass for all supported Python versions.

@GenevieveBuckley
Copy link
Collaborator

Here are some comments, although you might like to wait a bit for responses to this comment before trying it out.

  1. Here's an example where we alias a function for deprecation reasons.

sum is an alias of sum_labels

def sum(image, label_image=None, index=None):
"""DEPRECATED FUNCTION. Use `sum_labels` instead."""
warnings.warn("DEPRECATED FUNCTION. Use `sum_labels` instead.", DeprecationWarning)
return sum_labels(image, label_image=label_image, index=index)

Obviously, there's no need for a warning in this PR. Maybe the docstring could be as simple as """Alias of 'gaussian_filter'."""

  1. The second step is to add the alias function name to __all__

@K-Monty
Copy link
Contributor Author

K-Monty commented Feb 25, 2021

@GenevieveBuckley Thanks a lot for your comments! It seems like there is no objection(s) in the comment, so I'd given it another try.
By the way, do I also need to pay attention to the checks? For my last pull request, it said "All checks have failed".

Copy link
Collaborator

@GenevieveBuckley GenevieveBuckley left a comment

Choose a reason for hiding this comment

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

Thanks @K-Monty, I've added some comments for you.

The checks all seem to have passed with your most recent commit, so I wouldn't worry about why they failed earlier unless we start seeing that again.

dask_image/ndfilters/_gaussian.py Outdated Show resolved Hide resolved
dask_image/ndfilters/_gaussian.py Outdated Show resolved Hide resolved
@GenevieveBuckley
Copy link
Collaborator

Ok, this looks good to me now!

I'm going to leave this PR open for another week, just to give @jakirkham & others a bit more time for this discussion around design decisions. If there are no strong objections in a week, then we'll merge this here (feel free to ping me if I forget).

@GenevieveBuckley GenevieveBuckley merged commit 63543bf into dask:main Mar 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add alias 'gaussian' to point towards 'gaussian_filter' in ndfilters
2 participants