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.

Sign up for GitHub

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

Jump to bottom

Test gaussian alias #287

Merged
merged 4 commits into from
Mar 14, 2023
Merged

Test gaussian alias #287

merged 4 commits into from
Mar 14, 2023

Conversation

jakirkham
Copy link
Member

Test the gaussian alias to ensure it is counted in test coverage.

@jakirkham
Test gaussian alias
fa09e55 
Currently the `gaussian` function shows up as not covered by testing.
This provides some test coverage of it.
@jakirkham
Test gaussian with CuPy
7b377d6 
Include a CuPy test of `gaussian` as well.
@jakirkham jakirkham closed this Mar 13, 2023
@jakirkham jakirkham reopened this Mar 13, 2023
@jakirkham
Copy link
Member Author

Closing/reopening to restart CI (with latest changes from dask/main)

@jakirkham
Copy link
Member Author

rerun tests

@GenevieveBuckley
Copy link
Collaborator

I see what's happening - "gaussian" was included in the __all__ list for dask_image/ndfilters/_guassian.py, but it ALSO needs to be imported and in the list for the __init__.py file in ndfilters.

@jakirkham
Copy link
Member Author

Yeah I had missed this as well. Think this is now included. Though please let me know if you spot anything else missing

@GenevieveBuckley
Copy link
Collaborator

...I'm going to go for a coffee and leave this alone for 5-10 minutes. I think I'm getting in the way of you working on it at the same time 😆

@GenevieveBuckley
Copy link
Collaborator

Yeah I had missed this as well. Think this is now included. Though please let me know if you spot anything else missing

I guess it proves the point that there should have been a test included with the original PR, the oversight would have been very easy to catch then. But second best is doing it now, glad you're on top of it 😄

@jakirkham
Copy link
Member Author

rerun tests

@GenevieveBuckley
Copy link
Collaborator

All the CI checks are passing, looks good!

@GenevieveBuckley GenevieveBuckley merged commit 4871929 into dask:main Mar 14, 2023
@jakirkham jakirkham deleted the tst_gaussian branch March 14, 2023 01:41
@jakirkham
Copy link
Member Author

Thanks Genevieve! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jakirkham @GenevieveBuckley