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

Provide trait based opt in for Images API. #4

Closed
wants to merge 1 commit into from

Conversation

Tokazama
Copy link

@Tokazama Tokazama commented Aug 3, 2019

This is following up on JuliaImages/Images.jl#815.

The idea is to provide a bear bones interface for acting within the Images.jl ecosystem. It also aims to disrupt everything else as little as possible when adopted by other packages in the JuliaImages organization.

@johnnychen94
Copy link
Member

johnnychen94 commented Aug 3, 2019

I don't think it's a good idea to add these traits here. Instead, the most appropriate places for these are ImageCore.

image

Here's my thought on the difference between ImageCore and ImagesAPI:

  • ImageCore contains the real (fallback) implementation, and to avoid type piracy, downstream packages are not recommended to change the behavior of it unless it's extending this function to types it owns (e.g., ImageMetadata has ImageMeta). Directly or not, users must use ImageCore before using ImageMetadata.

  • ImagesAPI doesn't contain any fallback implementation, and it doesn't export any names. The only work it does is to define how data goes through these wrappers in a consistent way, and the only purpose of it is to avoid name conflicts. Users can choose to use only one of ImageNoise and DeepImageNoise because there's no default implementation here.

Of course, we can argue that ImagesAPI can hold the fallback implementation, but in that case, why do we need another "ImageCore"?

Perhaps the most key difference is that ImageCore is the backbone of Images.jl while ImagesAPI only provides a consistent interface at the user-experience level. By saying this, if it's all about JuliaImages, then ImagesAPI is unnecessary, it's useful only when a third-party developer wants to write a new application package as an alternative or extension to JuliaImages's package. And in that case, we want to make the user experiences of using two packages the same.


I'm not familiar with the practical usage of AxisArray, at least during my development of several packages I don't quite need this feature, so I don't have any comments on the implementation as long as it doesn't break the current codes 😄

@Tokazama
Copy link
Author

Tokazama commented Aug 3, 2019

Fair point. Would you like me to move this over to ImageCore.jl then?

@Tokazama
Copy link
Author

Tokazama commented Aug 3, 2019

Moved to JuliaImages/ImageCore.jl#86

@Tokazama Tokazama closed this Aug 3, 2019
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

Successfully merging this pull request may close these issues.

2 participants