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

Meta-issue: pending API changes for 2.0 #5439

Closed
grlee77 opened this issue Jun 22, 2021 · 27 comments
Closed

Meta-issue: pending API changes for 2.0 #5439

grlee77 opened this issue Jun 22, 2021 · 27 comments

Comments

@grlee77
Copy link
Contributor

grlee77 commented Jun 22, 2021

The "1.0" concept since changed to "2.0", so replace all 1.0 below with 2.0

Description

This issue is just to summarize some of the API considerations we need to make a decision on for the 1.0 release. Please edit and add any additional items I may have forgotten (most of these I copied over from the scikit-image API 1.0 "project" under the Projects tab).

Summary of various proposed Scikit-image API changes to consider for 1.0

Module deprecations

Function/module moves

Parameter removal

parameter/function renaming

Misc

@jni
Copy link
Member

jni commented Jun 23, 2021

@grlee77 a fantastic list! I would make one modification and two additions:

  • for 1.0 I think we should only have one name per region property. We can probably rethink the whole API anyway so that e.g. you get a dict rather than a list where the index has no relation to the label of that region. Regionprops is probably the most useful part of the library so let's think big!
  • All functions that take in x/y (transposed) coordinates should take axis-aligned (aka rc) coordinates instead. Example: warps and geometric transformations.
  • Audit our modules to cluster functions based on purpose and "interchangeability". For example, segmentation.watershed and segmentation.slic are "interchangeable", but segmentation.join is a different thing. The "big picture" in this case is to be sklearn-like in that it will enable easier interoperability between skimage and libraries replicating or extending the skimage API.

@jni
Copy link
Member

jni commented Jun 23, 2021

Regarding @stefanv's request to schedule a meeting, I will propose a doodle shortly, but my first proposal is that we have three meetings: Pacific, Atlantic, and Indian, so that we can all overlap at least once. Are people ok with this plan?

@stefanv
Copy link
Member

stefanv commented Jun 23, 2021

That works for me.

@mkcor
Copy link
Member

mkcor commented Jun 24, 2021

@grlee77 maybe this is the right place to add this item:

Parameter addition

  • Along with the deprecation of multichannel and its replacement with channel_axis, we probably want to also add a new channel_axis argument to any functions that always assumed a color image for input and thus did not have a multichannel argument previously. This would include many functions in skimage.color; what about elsewhere? See original conversation.

@grlee77
Copy link
Contributor Author

grlee77 commented Jun 25, 2021

Out of these issues, I see the issue of range-dependent default parameters (mentioned in #5439) to be one of the more complicated to address. A concrete example is the slow behavior of blob_doh for floats with range much greater than 1.0 (reported in #5224) due to a default threshold that is appropriate to floats scaled to [0, 1.0]. Getting this right everywhere will take a somewhat detailed audit.

@jni
Copy link
Member

jni commented Jul 1, 2021

I see the issue of range-dependent default parameters (mentioned in #5439) to be one of the more complicated to address. A concrete example is the slow behavior of blob_doh for floats with range much greater than 1.0 (reported in #5224) due to a default threshold that is appropriate to floats scaled to [0, 1.0]. Getting this right everywhere will take a somewhat detailed audit.

Yes, we'll indeed need to do an audit, but I don't think it's a deal breaker. In the worst case scenario we can ascertain data range in two passes.

@grlee77
Copy link
Contributor Author

grlee77 commented Jul 1, 2021

#2264 had proposed changing the default boundary mode from Gaussian back to the same default as SciPy's gaussian_filter. (Since skimage modes match numpy.pad conventions, this would be called symmetric rather than reflect, though)

@jni
Copy link
Member

jni commented Jul 1, 2021

Notes from the meetings are being edited at:

https://hackmd.io/A51uU56pTESLlhcKfH4lQg

Archival version at:

https://github.com/scikit-image/meeting-notes/blob/main/2021/july-api-meetings.md

should be updated after each meeting.

@grlee77
Copy link
Contributor Author

grlee77 commented Jul 8, 2021

I opened a PR (#5462) adding channel_axis to skimage.color functions per Marianne's suggestion. One other place that potentially adding channel_axis was previously discussed was for regionprops (see #4087), although there was some concern brought up there about that approach potentially requiring too much memory.

@grlee77
Copy link
Contributor Author

grlee77 commented Jul 8, 2021

In #5451 (comment), @rfezzani expressed a preference for also deprecating (or at least not expanding) the draw module which I don't think we discussed in the recent API meetings. Is this something we should be considering?

We do make use of a number of these for illustrative purposes in examples, so that would be a point in favor of keeping them. I think ongoing maintenance burden should be pretty low

# examples using shape functions
segmentation/plot_regionprops.py:from skimage.draw import ellipse
features_detection/plot_corner.py:from skimage.draw import ellipse
features_detection/plot_shape_index.py:from skimage.draw import disk
edges/plot_circular_elliptical_hough_transform.py:from skimage.draw import circle_perimeter
edges/plot_circular_elliptical_hough_transform.py:from skimage.draw import ellipse_perimeter
edges/plot_marching_cubes.py:from skimage.draw import ellipsoid
edges/plot_polygon.py:from skimage.draw import ellipse
edges/plot_line_hough_transform.py:from skimage.draw import line

# examples focused specifically on drawing shapes
edges/plot_shapes.py:from skimage.draw import (line, polygon, disk,
edges/plot_shapes.py:from skimage.draw import line_aa, circle_perimeter_aa
edges/plot_random_shapes.py:from skimage.draw import random_shapes

@stefanv
Copy link
Member

stefanv commented Jul 8, 2021

In #5451 (comment), @rfezzani expressed a preference for also deprecating (or at least not expanding) the draw module which I don't think we discussed in the recent API meetings. Is this something we should be considering?

I find skimage.draw very useful, so would prefer if we did not deprecate. What is the incentive for getting rid of it? There is no easy mechanism for replacing it as far as I know.

@rfezzani
Copy link
Member

rfezzani commented Jul 9, 2021

I think drawing is a job for the different visualization libraries (matplotlip, plotly, napari...)!

I agree that draw is less impacting then io or viewer, so it doesn't really hurt to keep it. But it looks more like a "util" module compared to the other modules of skimage 😕 ...

@stefanv
Copy link
Member

stefanv commented Jul 10, 2021

Note that none of those libraries draw directly to arrays very easily. The purpose of this module was to annotate frames of a video, for example.

@jni
Copy link
Member

jni commented Jul 15, 2021

I'm 👍 for keeping draw. It is in fact useful for creating coordinate index lists, which are in turn useful various analysis tasks. (Indeed we ourselves use draw.ellipse to make an elliptical footprint in morphology!)

@imagesc-bot
Copy link

This issue has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/request-for-comment-plans-for-breaking-changes-in-scikit-image-1-0/55386/3

@haesleinhuepf
Copy link
Contributor

Hi all,

I'm just curious: Are there plans for improving type-annotations of parameters in skimage 1.0? Or will parameter-name unification bring type hints implicitly? I'm asking because it might make it easier to explore the API, especially in an automated fashion. For example, if one wanted auto-generate user-interfaces from skimage functions (magicgui), type-annotations would be helpful.
There is two projects with similar aims where this was explored / attempted to exploit: @tlambert03's proto-skimage-widgets and clesperanto.

@grlee77
Copy link
Contributor Author

grlee77 commented Jul 21, 2021

Hi @haesleinhuepf, I think we will add type annotations eventually, but I don't have a specific time frame in mind. There was an initial start on this about a year ago: #4794, but I don't think there has been any recent progress. Now that NumPy and SciPy are a little farther along on these efforts, we can probably move forward a bit more easily.

@jni
Copy link
Member

jni commented Jul 21, 2021

Yup, relevant also @haesleinhuepf, NumPy's tensor typing meeting notes:

https://mail.python.org/pipermail/numpy-discussion/2021-June/081863.html

@grlee77
Copy link
Contributor Author

grlee77 commented Aug 22, 2021

Apparently we use scipy.ndimage boundary mode names in skimage.feature and skimage.filters, but numpy.pad style names in skimage.transforms and a couple of other isolated functions:
#2264 (comment)

Functions using scipy.ndimage mode names

    skimage.feature
        canny
        corner_kitchen_rosenfeld
        hessian_matrix
        structure_tensor

    skimage.filters
        compute_hessian_eigenvalues
        correlate_sparse
        difference_of_gaussians
        frangi
        gabor
        gaussian
        hessian
        hessian_matrix
        median
        meijering
        prewitt
        sato
        scharr
        sobel
        threshold_local
        unsharp_mask

    skimage.measure
        profile_line

Functions using numpy.pad mode names


    functions in skimage._shared.interpolation.pyx

    skimage.feature
        match_template

    skimage.restoration
        denoise_bilateral

    skimage.transform
        pyramid_expand
        pyramid_gaussian
        pyramid_laplacian
        pyramid_reduce
        rescale
        resize
        rotate
        swirl
        warp

    skimage.util.apply_parallel  (needs cleanup, also allows dask mode names)

@scikit-image scikit-image locked and limited conversation to collaborators Oct 18, 2021
@grlee77
Copy link
Contributor Author

grlee77 commented Feb 20, 2022

comments moved over from Discussion

rfezzani
on Nov 3, 2021
Can we look at this discussion from the perspective of the conclusion of the SKIP3 to rename the package skimage2 for the v1.0 (hope we will not have to create skimage3 for v2.0 sweat_smile)?
I mean, since we plan to release a new package, we don't really care about backward compatibility with scikit-image and we can break the API as much as we want (Remove skimage.io and skimage.viewer, use ndimage mode names...)

stefanv
It is not true that we no longer care about backward compatibility. We still want to make the minimal changes necessary to create a consistent API. Otherwise, we are generating unnecessary churn for our users.

rfezzani
on Nov 4, 2021
@stefanv, I don't see then why we need to create a new package skimage2... Why not keep going as we do actually with deprecation cycles? Let's just decide that the version after v0.19 will be v1.0 and we are good...

@matthew-brett, I think that the comparison with python 2 -> python 3 transition is not relevant since in that case both versions can't be used together. While in our case, skimage2 can be used alongside scikit-image. I mean that users can leave their scripts/packages depending on scikit-image as they are and only update the parts they need using skimage2.

@jni
on Nov 4, 2021
@rfezzani I interpret @stefanv's comment to mean: we don't want to change things just because we can, rather, we should change things where it makes sense, even though we have the freedom to change them. ie let's not go overboard for no reason, because even with a new package, users have muscle memory and it's good to have some similarity to 0.19 to orient them.

matthew-brett
on Nov 3, 2021
Hi,
On Wed, Nov 3, 2021 at 7:30 PM Stefan van der Walt @.***> wrote:
It is not true that we no longer care about backward compatibility. We still want to make the minimal changes necessary to create a consistent API. Otherwise, we are generating unnecessary churn for our users.
For example - in retrospect, one thing that made the Python 2 / Python
3 transition particularly difficult was the initial advice that people
treat Python 3 as a different language, and the idea that people would
switch their code-bases wholesale from 2 to 3. Luckily the changes
were not in fact great enough to make it impossible to have one
code-base for both versions, and that gradually became the default,
but I think you'd have to conclude, in retrospect, that the breakage
was too great, and more thought should have gone into the problem of
keeping code maintained for both versions for the medium term.
Cheers,
Matthew

matthew-brett
on Nov 4, 2021
Maintainer
@matthew-brett, I think that the comparison with python 2 -> python 3 comparison is not relevant since in that case both versions can't be used together. While in our case, skimage2 can be used alongside scikit-image. I mean that users can leave their scripts/packages depending on scikit-image as they are and only update the parts they need using skimage2.
Aha - interesting - I'm not sure I can think of an example of that
pattern - where people depend on the old and the new version at the
same time. That might be a difficult sell. And I wonder whether
people will do that in practice, or whether they'll just stick with
scikit-image.

@rfezzani
on Nov 4, 2021
> or whether they'll just stick with scikit-image.
That's what I'm afraid of with the introduction of skimage2 sweat

@jni
on Nov 4, 2021
I'm ok with this — this is equivalent to people pinning, which they would have done with SKIP3, and is generally what you advocate for them to do, right @rfezzani? smile They will upgrade when it is compelling to do so — hopefully that will be quick. =D

@rfezzani
on Nov 4, 2021
> ... this is equivalent to people pinning
You can see it this way laughing, with the slight difference that scikit-image dies when skimage2 is released...

@grlee77
Copy link
Contributor Author

grlee77 commented Feb 20, 2022

comment moved over from the discussion

Here is a new summary of which items above were already completed via deprecations for skimage 0.19 vs. those that are still remaining. If anyone wants to iterate on these via hackmd or similar, let me know and we can link to it from here:

Recently completed deprecations for skimage 0.19:

  • Deprecated multichannel in favor of channel_axis. Functions that previously assumed multichannel input, now have a new channel_axis argument that allows specifying which axis corresponds to channels.

  • The vast majority of functions supporting floating point inputs now preserve single vs. double precision rather than always promoting to double precision.

  • Consistently using footprint rather than selem to refer to structuring elements or kernels used in skimage.morphology and skimage.filters.rank.

  • Many regionprops were renamed to group similar attributes names together with the aim of improving discoverability for users.

  • Consistently using num_iter and max_num_iter everywhere for parameter names related to number of iterations or a maximum iteration limit, respectively.

  • Consistently use gray vs. grey in function names, modules and parameter descriptions

  • Consistently use disk vs. circle in function/parameter names referring to a filled circle

  • Consistently use out kwarg rather than an in_place kwarg when in-place computation is supported.

Proposed API changes surrounding skimage2

Remove the skimage.viewer module

napari is a more actively developed and more powerful third-party alternative.

Remove the skimage.io module ?

Still a question on whether to leave this in place as a thin wrapper around imageio or whether we remove it altogether and just ask users to rely on imageio directly.

Move RAG modules from skimage.future.graph to skimage.graph

Consistent default behavior of thresholding functions

We should make the functions in skimage.filters.threshold behave more consistently. Many currently return a numeric threshold value, but some return a thresholded image instead. It is likely that most users want the actual thresholded image, so that should be the default.

Drop support for outdated region property names.

We have some properties that have even been renamed twice, leaving a mess of legacy names that are still supported, but create confusion for users.

Potentially move thresholding functions to a new submodule

It is a bit odd to me that these "threshold" functions live in the skimage.filters module. A skimage.binarize module was proposed instead in: #2538

Use a consistent API (e.g. using radius) for the shaped footprint generation functions (disk, octagon, cube, ball, etc.)

These are currently a mess of size, width, height, m, n, radius, etc. #4536.

I would argue that we really only need a n-dimensional rectangle implementation rather than separate square, rectangle, cube, etc. Likewise an n-dimensional sphere (or more generally an n-dimensinal ellipse) rather than ball and disk.

I would argue that the shape is not quite right currently in disk/ball/ellipse generation functions and should be changed (needing adjustment by 0.5 pixels internally): https://github.com/scikit-image/scikit-image/discussions/5652

Potentially move shaped footprint generation functions to a new module

There was a proposal in #2538 to move the functions used for generating shaped footprints for morphology (disk, octagon, cube, ball, etc.) to a separate skimage.shapes module.

Consistently preserve the range of user-provided inputs

Some functions have a preserve_range kwarg with a default that is not uniformly the same. We had proposed just removing this kwarg from all functions and defaulting to the preserve_range=True behavior everywhere (#5281 (comment), #3373). A nice argument for not rescaling to [0, 1] range was given here: #3009 (comment)

Of the proposed API changes, this is one of the most involved as it involves auditing the behavior of functions and determining when changes to parameter defaults are required. For example, default settings for thresholds or regularization weights may currently rely on data being in the range [0, 1].

Making these changes should be good for end users, helping to avoid a very common source of confusion. The problem is that it will result in sudden changes to output for existing code as parameter defaults may have changed and internal rescaling would no longer occur. In terms of reproducible results of prior analysis, we would need to advertise widely that older code should pin to skimage<1 and that outputs need to be compared when updating existing code.

Rename img_as_* to rescale_to_*:

#1234.

All functions that take in x/y (transposed) coordinates should take axis-aligned (aka rc) coordinates instead.

https://github.com/scikit-image/scikit-image/discussions/5651#discussioncomment-1492411
"Example: warps and geometric transformations."

Audit our modules to cluster functions based on purpose and "interchangeability".

https://github.com/scikit-image/scikit-image/discussions/5651#discussioncomment-1492411
"The "big picture" in this case is to be sklearn-like in that it will enable easier interoperability between skimage and libraries replicating or extending the skimage API."

Several parameter naming changes to provide users a consistent API

These could proceed via a standard deprecation cycle as we have been doing all along.

  • Unify boundary extension mode names used across the library

Currently we have 21 functions that use boundary mode names compatible with scipy.ndimage while we have another set of 12 functions that use names compatible with numpy.pad. Filtering/morphology functions tend to use the scipy.ndimage names while those in restoration/transforms tend to use the NumPy ones. Out of these the subtle difference between reflect/mirror/symmetric can be quite confusing to users with reflect unfortunately being a valid name, but with different behavior in the two conventions!

The list of specific funtions following each convention is here: https://github.com/scikit-image/scikit-image/discussions/5651#discussioncomment-1492429

  • Consistently use workers for any parameter related to the number of threads/processes.

  • have skimage.draw functions use a center argument rather than row, col for 2D and row, col, plane for 3D.
    Restructuring of API #2538 (comment)

  • Use a consistent name for user-provided randoms seeds (currently have seed, sample_seed, random_seed, etc.)

  • Consistently image rather than img, arr, data, etc for image inputs.

  • Consistently use label_image as the parameter name refering to label images

  • Change output to out in skimage.filters.gaussian for consistency with other skimage functions supporting output to a user-provided array (older use of an in_place kwarg was already deprecated some places in 0.19).

  • resolve API inconsistency between remove_small_holes and remove_small_objects. API inconsistency between remove_small_holes and remove_small_objects #4003

Adding type annotations

Does not affect backwards compatibility, so would not have to be simultaneous with skimage2. Was requested by users, e.g.: https://github.com/scikit-image/scikit-image/discussions/5651#discussioncomment-1492426

@grlee77 grlee77 reopened this Feb 20, 2022
@mkcor
Copy link
Member

mkcor commented Oct 5, 2022

If anyone wants to iterate on these via hackmd or similar, let me know and we can link to it from here

@grlee77 yes! I think it would be convenient to have an interactive/collaborative document to keep track of all these items. I've started https://hackmd.io/f4_EjhYURxGvxqzaEjPeBQ if that's all right.

@lagru it might be useful to #6556 as well ^^

@stefanv
Copy link
Member

stefanv commented Oct 5, 2022

Also want to link to the new transition proposal being discussed.

@mkcor mkcor mentioned this issue Nov 5, 2022
9 tasks
@lagru
Copy link
Member

lagru commented Feb 10, 2023

Thanks @mkcor for starting that document. I've just pulled everything from #5439 (comment) into https://hackmd.io/f4_EjhYURxGvxqzaEjPeBQ?view. our wiki at https://github.com/scikit-image/scikit-image/wiki/API-changes-for-skimage2.

I intend to use it as a living document. Editing on HackMD has a lower barrier than versioned documents and is less scattered than issue / forum threads. So hopefully it's easy to keep it up to date.

I also have the idea to put down a list of guidelines that we can reference when we design our API going forward (not only for skimage2). I have several ideas I'd like to propose and I'm sure there will be more stuff coming up as we start thinking about typing our API. There will probably be a lot of discussion around such guidelines so that's looks like a SKIP could be in order.

Edit: moved document from HackMD to our wiki

@grlee77
Copy link
Contributor Author

grlee77 commented Feb 16, 2023

Thank you @mkcor and @lagru for helping organize these into the new HackMD doc!

@stefanv
Copy link
Member

stefanv commented Apr 7, 2023

canny's mode argument should not default to constant, since that usually results in an edge on the boundary.

@stefanv stefanv changed the title Meta-issue: pending API changes for 1.0 Meta-issue: pending API changes for 2.0 Sep 26, 2023
@stefanv
Copy link
Member

stefanv commented Sep 26, 2023

This issue has been summarized into https://github.com/scikit-image/scikit-image/wiki/API-changes-for-skimage2

@stefanv stefanv closed this as completed Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants