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

Sort out our dependencies and moving to version 1.0 #825

Open
timholy opened this issue Nov 22, 2019 · 10 comments
Open

Sort out our dependencies and moving to version 1.0 #825

timholy opened this issue Nov 22, 2019 · 10 comments
Milestone

Comments

@timholy
Copy link
Member

timholy commented Nov 22, 2019

For automatic registration we need to add upper bounds to all dependencies across JuliaImages. As some of you know from experience, without preparation this can cause a bit of a train wreck when it comes to Pkg's resolver. I just went through a whole bunch of the packages in JuliaImages and added CompatHelper actions, which currently is our best mechanism to determine when versions need to be bumped. For those of you who maintain packages, I may not have done this with yours, I only did this for ones where I feel like I have a reasonable "ownership" stake. On https://github.com/JuliaImages the packages are sorted by recency of modification, so you can easily see whether you might want to do the same for one of your packages.

Kristoffer Carlsson may have posted a useful script here but discourse seems to be down ATM so I can't easily check.

@timholy timholy changed the title Sort out our dependencies Sort out our dependencies and moving to version 1.0 Nov 22, 2019
@timholy
Copy link
Member Author

timholy commented Nov 22, 2019

Also, since changing bounds requires a bump in minor version number, I think it makes sense to make the transition throughout the whole JuliaImages stack to version 1.0:

  • semver says we should have done that a long time ago
  • discourse says we should
  • 1.0 could be psychologically important for grant proposals and papers

This does not mean that we can't make breaking changes in the future; package versions will just go to 2.x or higher.

Now is a good time, however, to propose any breaking changes. I'll post an issue in some packages, feel free to add new issues or add to those.

@johnnychen94
Copy link
Member

For packages that aren't well built at present(e.g., ImageDraw.jl, ImageMorphorlogy.jl), will it be too early to set up a 1.0 goal? I'm afraid that I'm currently too busy with my daily work to catch up with this schedule.

Another related question is: assume that Images.jl had bumped to 1.0, what should be done if we intend to bump a release of ImageTransformation.jl. According to SemVer, looks like every minor version bump in ImageTransformation.jl should also bump a minor version in Images.jl.

@timholy
Copy link
Member Author

timholy commented Nov 22, 2019

According to SemVer, looks like every minor version bump in ImageTransformation.jl should also bump a minor version in Images.jl.

Can you link to the relevant section? Once we get to 1.0, I imagine many [compat] sections might look like ImageTransformation = "1" and thus be "immune" from minor-version bumps. But of course we'll have to bump the major version if we want to make a breaking change.

I'm fine with waiting on things like ImageDraw. It would be nice to get ImageMorphology into 1.0 state since it's a dependency of Images itself. Or, we could drop it from Images and thus give it more time to mature.

@johnnychen94
Copy link
Member

johnnychen94 commented Nov 23, 2019

Once we get to 1.0, I imagine many [compat] sections might look like ImageTransformation = "1" and thus be "immune" from minor-version bumps.

For me, this is one way to break the rule of SemVer. Since ImageTransformation.jl is purely reexported by Images.jl, a backward-compatible functionality in the former is also a backward-compatible functionality in the latter. Doing like this is okay if we're still in v0.x, but I think we need to think it more careful if we want to head into v1.0.

Although every package is affected by its upstream dependencies, if we treat Images.jl as a toolkit level release, there should be more guarantees on its internal stability. Making a regular release schedule (e.g., every two months) that updates version changes of packages that contribute to the Images API specifications seems more reasonable to me. By saying this, I mean we could set FixedPointNumbers = "1" since it's doesn't change the API, but we need to specify at least ImageTransformation = "1.0" since by bumping ImageTransformation.jl from v1.0 to v1.1, there must be some non-trivial changes to users of Images.jl.

@johnnychen94
Copy link
Member

I'm not fully available until Jan 2020. It would be great if you can provide a freezing date so that I can estimate what contributions I can make.

@johnnychen94
Copy link
Member

Perhaps adding ImageBinarization.jl into Images.jl as a part of v1.0 can be taken into consideration. @zygmuntszpak

@timholy
Copy link
Member Author

timholy commented Nov 23, 2019

For me, this is one way to break the rule of SemVer.

I'm not sure I agree. If your package depends on specific features in ImageTransformations you can always add a [compat] entry for it even if you're also declaring Images as a dependency.

It would be great if you can provide a freezing date so that I can estimate what contributions I can make.

I'm flexible. I'd feel better if we get the FixedPointNumbers/Color* transition done sooner than that, but the JuliaImages packages could come out on whatever schedule it takes. I think the main thing is to make some decisions about what is necessary to accomplish before we hit 1.0.

1.0 does not imply that the packages are perfect. In many ways we've effectively been at 1.x for years, and are currently abusing semver. I fully intend to keep making these packages better after they hit 1.0. But given that we haven't yet made that transition, it is a chance to make some breaking changes, and I'd like to see what that list looks like.

@zygmuntszpak
Copy link
Member

Thank you Tim for taking the time to set a direction for the project. I reckon it makes sense to transition ImageCore and FixedPointNumbers/Color* to 1.0. I've worked a lot with the Images ecosystem over the last year and these foundational packages have been a reliable bedrock to build upon.

It would be great if we could move most algorithms that are in Images into separate packages. I think its a bit awkward to have a handful of algorithms implemented in Images while many others are reexported from other packages.

I've implemented many contrast related algorithms in ImageContrastAdjustment.jl. The package covers all that is currently in exposure.jl with the exception of clahe. I have an almost complete implementation of clahe that doesn't involve resizing the image into something that is divisible by 2 like the current implementation in exposure.jl, but I haven't gotten around to finalizing it just yet. The package also includes comprehensive documentation, so it should be an easy drop-in replacement.

The ImageBinarization.jl package includes the functionality of otsu_threshold and yen_threshold (and many others). It also has comprehensive documentation so is also a convenient drop-in replacement.

More recently I have been working on a package, ImageComponentAnalysis for analyzing and filtering connected-components. I registered it because I need it for another project, but it is still a work-in-progress (the documentation is also sparse). I used PlanarConvexHulls.jl as a dependency, and so I could easily export the function that computes a convex hull. Hence, we have a potential replacement for the convexhull.jl file in Images. I also copied and exported label_components function from Images.jl.

The ImageComponentAnalysis package takes as input a labelled array of connected components, and returns a DataFrame where each row corresponds to a component, and each column corresponds to some attribute of the component, such as its bounding box, area, perimeter etc.
When combined with DataFramesMeta.jl or Querly.jl one has a very elegant way of filtering components based on their attributes.

All of these packages follow a functor style API so they have a consistent feel about them.

When developing pacakges I've often needed functions such as minfinite, minfinite_scalar, and sentinel_min (and its variants). I proposed that these functions are also moved into a separate package (maybe ImageCore?). This would take us a step further to thinking of Images as something that pulls in a set of curated packages. I'd prefer that packages that provide functionality to Images not depend on Images itself.

@timholy
Copy link
Member Author

timholy commented Nov 30, 2019

These sound like great changes. I basically agree with everything you said.

One nuance worth discussing: what to do with sections of code like this? (This section of code implements restrict methods that update the pixelspacing for array types that support pixelspacings different from 1). There are different approaches, each with their own advantages and disadvantages:

  1. put such code inside a @require block in the package (ImageTransformations) that defines restrict. Advantage: ensures that restrict always behaves as it should even for the most minimal set of packages that you load. Disadvantage: code inside @require blocks cannot be precompiled, ensuring longer latencies.
  2. leave such glue code in packages like Images.jl that load all the dependencies. (Advantages and disadvantages are essentially the mirror image of the ones above---note that you get the "good" behavior of restrict only if you load Images itself.)
  3. create dedicated glue packages whose only purpose is to precompile big blocks of code (like this one) when both sets of requirements are available. For example, in ImageTransformations we might add @require AxisArrays=<uuid> using ImageTransformationsAxisArraysGluePkg. Advantage: as in 1, but with working precompilation. Disadvantage: a possible proliferation of packages whose only purpose is to implement glue methods.

We don't currently have a much in the way of precompile statements, but I am already working on adding them---locally I have precompile files for FixedPointNumbers, ColorTypes, and Colors, and I'm working my way up the hierarchy. In the longer run that should significantly reduce latency (JuliaImages version of "time to first plot").

@johnnychen94
Copy link
Member

Hmmm, I think this issue should be closed now with the existence of #792 and #841

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

3 participants