search

JuliaImages / Images.jl

An image library for Julia
http://juliaimages.org/
Other
535 stars 142 forks source link

Countdown to the new Images.jl #542

Closed timholy closed 7 years ago

timholy commented 8 years ago

There have been justifiable questions about "what exactly is happening?" so let me post my TODO list here (including completed items, with an explanation of purpose):

Things that could be done before or after:

Longer-term future: implement lots of missing functions. Other BSD-licensed projects should be used as important inspiration.

timholy commented 8 years ago

Thanks! I updated the gist, now it should fetch automatically.

timholy commented 8 years ago

One thing I forgot to ask: do folks think that all the documentation should be moved to http://juliaimages.github.io/latest? (Meaning, should ImageCore, ImageFiltering, etc. continue to maintain their own documentation?) Either way, http://juliaimages.github.io/latest/conversions_views.html#Conversions-vs.-views-1 is largely redundant with https://juliaimages.github.io/ImageCore.jl/latest/views.html#Views-1. Any thoughts on this would be appreciated. Originally I intended to just link the page from ImageCore in to juliaimages.io, but there are technical difficulties with that, and once I thought about just copying I came to the conclusion that some of the groundwork laid on earlier pages led to me wanting to explain things slightly differently. So if you think we should have only one of them, concrete changes for suggestions about which to keep or how to merge them would be appreciated.

Evizero commented 8 years ago

Personally I think the following would make sense:

timholy commented 8 years ago

The principle seems sound. But sometimes the dividing line between the two is unclear, perhaps especially to me (given that I spent so much time thinking about the technical details). To pick just one example: is http://juliaimages.github.io/latest/conversions_views.html#Sharing-memory:-an-introduction-to-views-1, pointing out the differences between a copy and and view, a "detail" or "necessary"?

My favored approach would be for others to cut/paste the docs and move them appropriately, since others have a better "outsider" perspective, but I'm happy to do it particularly if folks give me sufficiently detailed instructions.

tbreloff commented 8 years ago

My vote (not that I have one to cast) would be to have to docs all in one place and dedicate a page to each package to explain how it fits into the ecosystem and any peculiars one would need to know if they were to use it in isolation. Then you could link directly to that page from the respective repo.

On Tuesday, October 18, 2016, Tim Holy notifications@github.com wrote:

The principle seems sound. But sometimes the dividing line between the two is unclear, perhaps especially to me (given that I spent so much time thinking about the technical details). To pick just one example: is http://juliaimages.github.io/latest/conversions_views.html# Sharing-memory:-an-introduction-to-views-1, pointing out the differences between a copy and and view, a "detail" or "necessary"?

My favored approach would be for others to cut/paste the docs and move them appropriately, since others have a better "outsider" perspective, but I'm happy to do it particularly if folks give me sufficiently detailed instructions.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/timholy/Images.jl/issues/542#issuecomment-254451693, or mute the thread https://github.com/notifications/unsubscribe-auth/AA492jT0L3eXLo4jNzlND2djWtXJGlM7ks5q1I3fgaJpZM4Jjs9f .

timholy commented 8 years ago

You definitely have a vote to cast! EDIT: I think this idea too has merit. "local" vs "global" may be determined in part by how easy/difficult it is to explain one piece in isolation from any other piece.

tbreloff commented 8 years ago

:)

Also... I could throw together a meta spec (for MetaPkg) for the transition if you want, to avoid making users copy/paste that long gist to setup. I want to help, but weary of pushing it on anyone considering the tension over package management.

On Tuesday, October 18, 2016, Tim Holy notifications@github.com wrote:

You definitely have a vote to cast!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/timholy/Images.jl/issues/542#issuecomment-254465529, or mute the thread https://github.com/notifications/unsubscribe-auth/AA492k1vK_YdhOxMga_ls3DHXRsHLT8Mks5q1JvIgaJpZM4Jjs9f .

timholy commented 8 years ago

I haven't played with MetaPkg yet. Very cool, and seems like a systematic solution to precisely the problem my gist was designed to address. I'd be thrilled!

tbreloff commented 8 years ago

Exactly. My needs were for managing the many packages of JuliaPlots, JuliaML, and the GLVisualize universe, many of them not registered.

On Tuesday, October 18, 2016, Tim Holy notifications@github.com wrote:

I haven't played with MetaPkg yet. Very cool, and seems like a systematic solution to precisely the problem my gist was designed to address. I'd be thrilled!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/timholy/Images.jl/issues/542#issuecomment-254469409, or mute the thread https://github.com/notifications/unsubscribe-auth/AA492hAJTKYDUvZns_P485Lwk-vL5-D6ks5q1KAlgaJpZM4Jjs9f .

bjarthur commented 8 years ago

would be nice to have more info in each package's README. i code in a terminal and it's nice when packages contain at least a list of exports therein so that one can just say ?JLD, for example, for a quick jog of the memory.

it's also very important to make the "description" (eg for ImagesCore it is currently "Julia types for representing images") informative. because, this and the name of the package are all that's listed at http://pkg.julialang.org. in such a constrained space, i think the word "Julia" is not useful.

tlnagy commented 8 years ago

I agree with @tbreloff here. One centralized place for documentation would be better. That way you don't have jump back and forth between different sets of docs. I would imagine that the less users are exposed to separate Image packages the better.

tbreloff commented 8 years ago

I took the liberty of adding a JuliaImages meta spec in MetaPkg. I tested it on my mac laptop, and everything seemed to work, but you should verify yourself (make sure all your work is pushed up because purge will do Pkg.rm and also delete cache/lib/trash files that are left around).

Instead of using that gist linked to above, you should be able to update with the commands:

using MetaPkg
MetaPkg.purge("JuliaImages")
meta_add("JuliaImages")

Then you should be all set up on the "release" path of the gist.

If you want to do the "fixed-renaming" branch:

meta_checkout("JuliaImages", "fixed-renaming")

and if you want to go back to the "release" afterwards:

meta_free("JuliaImages")

I didn't do the "images-next" path yet.. figured I'd get your input first. I made a gist with the full expected output from the REPL.

timholy commented 8 years ago

This looks awesome.

I have a bunch of not-pushed branches in quite a few of those packages, and I'm hopping on a plane this morning, so it might take me a bit of time to prepare the groundwork for testing this. But I really like how this looks as a way of managing an interwoven set of packages!

tknopp commented 8 years ago

I am a little late on this show, but would like to ask where all the metadata handling has gone (pixelspacing and so on) This was a very prominent feature in the stable branch of Images and in our internal codebase almost anything is an image object for exactly that reason. We also attach quite some additional metadata in image objects.

rsrock commented 8 years ago

Tim, would you mind opening an issue over on QuartzImageIO with some guidance on what needs to be updated? I haven't had a chance to try this out yet, but I hope to do so soon. Thanks.

timholy commented 8 years ago

@tknopp, it's now called ImageMeta. See ImageMetadata.jl. However, things like pixelspacing are no longer encoded in the dictionary; you should use AxisArrays (with a bit of extra functionality provided by ImageAxes). All these packages are loaded when you say using Images, however. I'm especially interested in transitions like the one you're describing, so if you have more than a little difficulty with the transition please let me know: the whole point of this beta period is to find out the pain points, and I can help smooth the pain.

@rsrock, will do.

tknopp commented 8 years ago

Ok, thanks. Not sure when we get to this but we will try. I ping my colleague @hofmannmartin to be aware of the Images transition. Since we also set the pixelspacing using im[:pixelspacing] it seems that we will need some transitioning.

timholy commented 8 years ago

The safer thing has always been to say pixelspacing(im), but perhaps this wasn't emphasized strongly enough. EDIT: and that will still work just fine (and without a deprecation warning), if you're using an AxisArray or ImageMeta that wraps one.

tlnagy commented 8 years ago

@timholy I see you that you might move the Overlay stuff out. Would that be the recommended way of handling multichannel images or should something else be used instead?

tknopp commented 8 years ago

That certainly has been emphasized. There are these annoying people not carefully reading the documentation... Sorry my workflow is sometimes a little exploratory.

timholy commented 8 years ago

@tlnagy, that's in fact already done, see http://juliaimages.github.io/latest/conversions_views.html#Using-colorview-to-make-color-overlays-1 and redundantly described in https://juliaimages.github.io/ImageCore.jl/latest/views.html#View-types-defined-in-ImageCore-1. That allows you to easily fill in channels for red, green, and blue (and you can do magenta/green with colorview(RGB, m, g, m)). If that's not flexible enough, I suspect that better than the old approach would be to define a custom color type, e.g., the 2-channel color

immutable GFPtdTomato{T} <: Color{T, 2}
    gfp::T
    tdTomato::T
end

along with a function that can convert such "colors" to RGB.

timholy commented 8 years ago

@tknopp, now I also see you said "set the pixelspacing". Yes, it might be a little more work, sorry---if you find it too annoying then let's talk about a convenience function, e.g.

im = imageaxes(buf, pixelspacing=(1mm, 1mm))

or something.

As consolation for any pain, among the benefits of having it be more "structured" now is that it won't be lost when you index like im[5:100, 40:80]; formerly that lost both the pixel spacing and the orientation, which was just evil.

tlnagy commented 8 years ago

Thanks! The ImageCore documentation is quite thorough. It does mention StackedView on several occasions, but I can't seem to find any docs on that.

timholy commented 8 years ago

The balance of advice above was to centralize the docs at JuliaImages, so the final product may look more like what's there than the ImageCore docs.

StackedView is kinda internal; if you use colorview(RGB, r, g, b) then you don't need to touch it. Briefly, StackedView takes m×n arrays a, b, c and creates a 3×m×n view of them.

tknopp commented 7 years ago

Is there some documentation how to combine ImageMeta objects with AxisArrays? I would like to start with an object that is a close to the current Image object and start from there.

timholy commented 7 years ago

Sure, they're completely orthogonal: just use an AxisArray for the array argument of ImageMeta. There's already some special-purpose code in place to look for this case, e.g., https://github.com/JuliaImages/ImageMetadata.jl/blob/b8e5c162b71babe457b56669e4cb8530964407f3/src/ImageMetadata.jl#L71-L73.

tknopp commented 7 years ago

So, a typical usage of Images.jl was

https://github.com/simonster/NIfTI.jl/blob/2f217aac10d0e48e77eeb437e82a11c203dc2255/examples/mriview.jl#L9

What is your advice to handle this in the future. I will try playing around with this myself, but maybe it would be good to have this particular case discussed in the documentation somewhere.

And whats about the other image loader, are the already ported to the new Image infrastructure?

timholy commented 7 years ago

In case you haven't seen it, https://juliaimages.github.io/ImageAxes.jl/latest/ might help.

Simple (ignoring pixel spacing): ImageMeta(AxisArray(colorview(Gray, ni.raw), :x, :y, :z, :time))

There isn't quite as simple an approach for handling the pixel spacing (this is exactly the kind of feedback I'm looking for), but it is more flexible:

ImageMeta(AxisArray(colorview(Gray, ni.raw), Axis{:x}(rngx), Axis{:y}(rngy), Axis{:z}(rngz), Axis{:time}(rngt)))

where rng[xyzt] is a range that describes the positions of the different coordinates, e.g., 0:2:284 if the pixels are 2mm and go from 0mm to 284mm. (If you want to attach real units, currently Julia's ranges are kinda problematic, but see Unitful.jl, Ranges.jl and https://github.com/JuliaLang/julia/pull/18777.)

Do you need an easier way to specify pixel spacing? The default AxisArray behavior is very flexible (it lets you specify the origin) but maybe somewhat verbose.

timholy commented 7 years ago

Note you don't really need the ImageMeta here, everything is encoded by the AxisArray...one of the main points of the redesign was to encode everything having to do with orientation in the type system so that A[:, 5, :] can preserve the names of the correct axes without hammering performance.

tknopp commented 7 years ago

Hm, this is of course flexible but also pretty complicated. The pixelspacing is one of the most important things since in our (tomographic) imaging method we have anisotropic voxels. Furthermore I use two custom fields "offset" and "rotation" within the image object that allow to define the precise coordinate system in space. With this I can take two Image objects measured with two different tomographs and overlay them doing the appropriate registration/regridding/image fusion. The rotation field is necessary since MRI scans can be arbitrarily angulated.

tknopp commented 7 years ago

So if I see it correctly I will have to but the offset into the ranges, but the rotation will remain an ImageMeta field.

timholy commented 7 years ago

Well, offset is exactly what you can encode in the AxisArray ranges. Might want a simpler approach, how about

A = AxisImage(data, (:x, :y, :z, :time), (dx, dy, dz, dt), [(x0, y0, z0, t0)])

?

As for rotation goes...that would either have to be in the metadata, or possibly you could use a TransformedArray from AffineTransforms.jl. That package will be deprecated by CoordinateTransforms.jl, however, but I can't remember the current status.

tknopp commented 7 years ago

Yes, such a syntax would be definitely a nice convenience constructor, so thumbs up.

timholy commented 7 years ago

Oh, and to answer an earlier question, yes, all the IO packages have been updated too if you check things out as described in https://github.com/timholy/Images.jl/issues/542#issuecomment-254059092. That gist has been updated several times, so you may want a fresh copy.

tknopp commented 7 years ago

Ok thanks.

Its quite interesting to see how this restructuring will change the workflow. Before, all parameters such as pixelspacing where just metadata and I extracted the underlaying array before giving it to a hot loop. Maybe this won't be necessary anymore if the type stability is preserved.

timholy commented 7 years ago

Yes, the idea is you just pass an AbstractArray to whatever you do. That way you never have to make choices between performance and "self-documentation." It should be just as fast as an Array.

Could you open an issue on ImageAxes with https://github.com/timholy/Images.jl/issues/542#issuecomment-270742133, so I don't forget?

timholy commented 7 years ago

Added to AxisArrays (need to be running master).

timholy commented 7 years ago

Closed by #577