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

Rename the input argument to image in the ndimage functions #117

Merged
merged 46 commits into from Aug 4, 2019

Conversation

akhalighi
Copy link
Contributor

@akhalighi akhalighi commented Jul 13, 2019

This PR implements the following:

  1. The wrappers for functions ported from scipy.ndimage are modified to rename the input argument to image.

  2. The _get_docstring utility function for filters is modified to:

    • replace the input keyword with image when porting docstrings.
    • drop Examples from the original docstring in scipy.ndimage since they most probably will be invalid for the dask-image wrappers.

Contributes to #101

@GenevieveBuckley
Copy link
Collaborator

This is great! 😄

We also need to change the labels=None keyword argument to be label_image=None, and then do the same kind thing you've just done for the _get_docstring utility function. Is that something you'd like to do as well, or do you prefer someone else do that before merging the PR?

@akhalighi
Copy link
Contributor Author

This is great!

We also need to change the labels=None keyword argument to be label_image=None, and then do the same kind thing you've just done for the _get_docstring utility function. Is that something you'd like to do as well, or do you prefer someone else do that before merging the PR?

@GenevieveBuckley sure, it sounds fun. I will give it a shot!

@GenevieveBuckley
Copy link
Collaborator

FYI: I'm to integrating the changes we made to our CI builds (so we can see the tests pass on your branch). I might also merge this in soon, and leave the labels --> label_image change open for a separate PR.

@GenevieveBuckley GenevieveBuckley merged commit 7e8932b into dask:master Aug 4, 2019
@GenevieveBuckley
Copy link
Collaborator

🎉 Yay, merged! 🎉
Thanks so much @akhalighi, your contribution is very useful to have - makes things a lot nicer :)

@jakirkham
Copy link
Member

Thanks @akhalighi for working on this and @GenevieveBuckley for the review!

@jakirkham
Copy link
Member

Also just realized I hadn't opened the SciPy issue about renaming input in these functions. Have just now done so. ( scipy/scipy#10592 )

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.

None yet

3 participants