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

skimage.filters: forward and compute_hessian_eigenvalues should be public API? #6024

Closed
grlee77 opened this issue Nov 9, 2021 · 3 comments
Closed
Milestone

Comments

@grlee77
Copy link
Contributor

grlee77 commented Nov 9, 2021

Description

The file skimage/filters/lpi_filters.py defines the class LPIFilter and functions forward and inverse, but for some reason forward is not part of the public API listed in the filters module's __init__.py. This forward function was also not in the init's __all__ prior to the introduction of the lazy loading PR, so I don't think that PR is to blame.

Isn't this an oversight? I don't see why we would have inverse be public, but not forward. On a related note, forward and inverse are not very descriptive function names in that there is no indication what type of filter these apply to! Perhaps with revisiting the API for these two for the 1.0 release.

On a similar note ridges.py defines a function named compute_hessian_eigenvalues that used to be imported to the top-level skimage/filters/__init__.py, but was also missing from __all__ and is now not available via lazy loading.

@grlee77 grlee77 added this to the 0.19 milestone Nov 9, 2021
@grlee77
Copy link
Contributor Author

grlee77 commented Nov 13, 2021

Neither of these functions has standalone test coverage, so perhaps not intended to be public API?

compute_hessian_eigenvalues appears to be used internally by meijering, sato and frangi, but is not otherwise used anywhere else in the tests.

@stefanv
Copy link
Member

stefanv commented Oct 11, 2022

Re lpi interface, see also #6418

@lagru
Copy link
Member

lagru commented Oct 11, 2022

compute_hessian_eigenvalues was never intended to be part of the public API. May cause breakage for a small number of users but @stefanv thinks that is acceptable. #6418 addressed the other concern.

@lagru lagru closed this as completed Oct 11, 2022
@jarrodmillman jarrodmillman modified the milestones: 0.21, 0.20 Jan 19, 2023
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

No branches or pull requests

4 participants