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

single precision support in skimage.filters #5354

Merged
merged 48 commits into from
Sep 3, 2021

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Apr 26, 2021

Description

This PR contains the commits from #5353, so that one should be reviewed prior to this (float32 support Hessian functions in skimage.feature were required in the ridge filters implementation here).

Checklist

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.

@grlee77 grlee77 added ⏩ type: Enhancement Improve existing features 📜 type: API Involves API change(s) labels Apr 26, 2021
@pep8speaks
Copy link

pep8speaks commented Apr 26, 2021

Hello @grlee77! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 6:80: E501 line too long (80 > 79 characters)

Line 21:80: E501 line too long (80 > 79 characters)

Line 118:80: E501 line too long (81 > 79 characters)

Comment last updated at 2021-08-24 23:04:51 UTC

@grlee77
Copy link
Contributor Author

grlee77 commented Jun 30, 2021

Hi @rfezzani, thanks for reviewing this previously. I fixed conflicts and addressed your comment about the global eps variable.

Since the last time you reviewed this, I added single precision support to the three threshold_* functions that return arrays (see commits 42c9a94 and 5ebcf52).

Also, it could go in a separate standalone PR if preferred, but there are a couple of minor fixes to prior single precision PRs in fbb1df9 (a dtype, not an image should be passed to _supported_float_type).

@rfezzani
Copy link
Member

rfezzani commented Jul 1, 2021

Thank you @grlee77, a second approval and we are good to go 😉 !

@grlee77 grlee77 added this to the 0.19 milestone Jul 8, 2021
@grlee77
Copy link
Contributor Author

grlee77 commented Sep 2, 2021

This is the only one of the single precision PRs that has not yet been merged. It has one approval now and the changes made here are consistent with the already merged PRs in this area. Given that, I would be inclined to merge it as is rather than ask others to spend substantial time doing additional reviews. Are there any objections to this?

@rfezzani
Copy link
Member

rfezzani commented Sep 2, 2021

... Are there any objections to this?

Not from my side ^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏩ type: Enhancement Improve existing features 📜 type: API Involves API change(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants