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

Move Shepp-Logan phantom to TestImages.jl? #899

Closed
stillyslalom opened this issue Jun 26, 2020 · 8 comments
Closed

Move Shepp-Logan phantom to TestImages.jl? #899

stillyslalom opened this issue Jun 26, 2020 · 8 comments
Milestone

Comments

@stillyslalom
Copy link
Contributor

It seems a bit out of place in algorithms.jl.

function shepp_logan(M,N; highContrast=true)

@johnnychen94
Copy link
Member

johnnychen94 commented Jun 26, 2020

A good idea to me. I've thought about this once a while, but just can't come up with a compatible API to the current testimage usage.

Probably would just move them around.

CRef: JuliaImages/ImageReconstruction.jl#4

@stillyslalom
Copy link
Contributor Author

If there's not a good way to shoehorn it into a testimage function call, TestImages.jl can easily have more than one exported method.

@johnnychen94
Copy link
Member

johnnychen94 commented Jun 26, 2020

I'm good to this, mind to make a PR?

Examples can be found at JuliaImages/ImageMorphology.jl#28 and #895

Doing this would introduce TestImages as a dependency to Images.jl, does this sound good to you? @timholy (Or just add a deprecation warning about deleting it in Images.jl?)

Instead of reexport TestImages.shepp_logan in Images.jl, we should just add a deleting warning about this, and redirect users to TestImages.

@johnnychen94 johnnychen94 added this to the v1.0 milestone Jun 26, 2020
@timholy
Copy link
Member

timholy commented Jun 26, 2020

I like the idea of moving it. We would have to do this in a way that is non-breaking, though, unless we bump minor or major versions.

@johnnychen94
Copy link
Member

For clarification, I was suggesting (1) copying it to TestImages, (2) add a deprecation in Images.jl, and finally (3) deleting it in Images@v1.0.0

@johnnychen94
Copy link
Member

johnnychen94 commented Jun 26, 2020

One way to make it compatible with the current testimage API is to redirect testimage("phantom", size=(256, 256), high_contrast = true) to its real generating function.

Note that "phantom" is the fullname of this image. No extension required.

@johnnychen94
Copy link
Member

johnnychen94 commented Jul 22, 2020

Some update on this:

shepp_logan is reimplemented in TestImages.shepp_logan (JuliaImages/TestImages.jl#77) and Images.shepp_logan will be removed in a future release (possibly v1.0.0)

@timholy
Copy link
Member

timholy commented Jul 23, 2023

This was done for the 0.25 release.

@timholy timholy closed this as completed Jul 23, 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

3 participants