Propsed Code Refactoring

[glennsweeney]

I've been starting developing a fork of this library, and working with the single monolithic file is frustrating for expansion. Would there be interest in refactoring the library into a more modular organization? I'm happy to put in the work; just looking for input if this is something the maintainers would support or not.

[StefanKarpinski]

The simple approach, if there's interest, is to just use include and split into logically related files still with a single big module. The more complex approach, which may or may not be warranted, is to use separate submodules for different color spaces.

[glennsweeney]

I'm not sure that distinct submodules are necessary; each color space shouldn't grow to be more than a few hundred lines at most. However, as more color spaces are implemented (and more conversions appear), Color.jl is going to get pretty crowded. I was thinking about moving types, conversions, and utilities out into separate files as a rough separation. However, if more fine grained modularity is desired, it could certainly be done.

[glennsweeney]

One other option is to leave Color as-is, let it serve general colorspace transformations, and move a lot of the "hardcore" color science into a separate Colorimetry module. I'm not sure what is best for the language and its users. It does create the headache of having to search for color-related functionality in two different places, but it means someone unfamiliar with color science isn't overwhelmed by the diversity.

[dcjones]

I'm in favor of splitting Color.jl into separate files. It has gotten a little large and diverse. I don't see a need to split things into submodules.

At this point I'd oppose splitting Color into simple and advanced packages. I agree that this package could get overwhelming for someone wanting to do some simple color manipulation, but that's something we could solve with improved documentation.

[glennsweeney]

Sounds good. I went through the library and came up with what seems like a reasonable refactor to me:

• Spaces
• Conversions
• Utilities (Hex code output, CIE table lookups, etc)
• Algorithms (white balancing, colorblindness simulation, etc)
• Color Differences
• Palettes
• Colormaps
• Display (swatches)

I'm not really qualified to discuss any of the color palette and colormap code; I haven't spent the time with it. Is there a better way to build that section?

I also chose to put color differences separately, because I plan on providing implementations for many more of the standard color difference equations used in various industries soon.

Thoughts?

[dcjones]

This sounds pretty good.

@natj wrote the color palette and colormap code and may have an opinion on how that should be organized, e.g., wether it should be split up at all. I'm inclined to put that all, along with distinguishable_colors in one file.

[natj]

Yeah, I would also vote for keeping all the palette and colormap things together. At least I would just make one colormaps.jl (or something like that) and put

• distinguishable_colors
• sequential_palette
• diverging_palette
• colormap
• linspace (?)

all there. Also the arrays at the bottom of color_names.jl could/should be separated into their own file or be incorporated into this colormap.jl (or whatever). At the moment there are only few predefined maps, but I plan to add more when I have time.

[glennsweeney]

It's definitely not ready for a pull request yet (unless you would like it), but I did an initial attempt at a refactor over on my fork at https://github.com/glenn-sweeney/Color.jl/tree/refactor. I don't know how to suggest it to be pulled to a branch other than master; still new to GitHub.

I tried to test as much as I could as I went, but not everything is bulletproof yet. However, it is hopefully a point to start talking to see if this still looks like a good idea or not.

The general idea was to keep all types in color_spaces.jl. Then, all early code needed across several files (as well as things that didn't seem to have a home) ended up in color_util.jl. In theory, all of the other files should be independent.

I also made an attempt to move large data into files prefixed with data_*.jl.

Thoughts?

[natj]

It is just my opinion and impression of the unspoken rules but why have a prefix of color_ for everything? I think for e.g. utils.jl or utilities.jl (instead of color_utils.jl) would be more according to the common naming conventions of julia repositories. This is of course just nitpicking on tiny details but it's something that caught my eye.

[natj]

Few notions more:

• I would move Beziereand invBeziere functions to utils instead of algorithms as they are really just common functions and have nothing to do with color calculations in general.
• These two definitions:

# Small definitions to make color computations more clear
+(x::LCHuv, y::LCHuv) = LCHuv(x.l+y.l,x.c+y.c,x.h+y.h)
*(x::Real, y::LCHuv) = LCHuv(x*y.l,x*y.c,x*y.h)

I would just leave where they were. That is near the sequential_palette function as they are made only for this one function.

We should also give a thought to other functions inside utils and algorithms if they are more of a general functions or really just custom made for some function in specific. In the latter case I think they should be then presented with the main function to make the code more readable.

[m-lohmann]

I was already thinking about refactoring along the same lines as you, Glenn. I’ll have a look after you’re done. I’m also working on a couple of other things, trying to transfer my homebrew MATLAB color science stuff to Julia, like a small database of spectra (around 40 measured or reconstructed spectra from the Gretag MacBeth ColorChecker and others), the later versions of the DIN99 color space (DIN99a to the latest DIN99o color spaces) and so on. Furthermore the standard observer data need to be updated or extended. One version is missing completely. I did not check if the version that is there is the 2° or 10° observer, but one of those versions is missing. But you need the 10° observer for color spaces like CIELUV, CIELAB, DIN99 etc. On top of that I’m also working on adding the latest 2007 CIE colorimetric standard observers based on the Stiles and Burch and further data from the Center of Image research (CIS)

One little remark: It’s Bezier , not Beziere. The name of the inventor was Pierre Bézier. These functions should be named properly, I think.

[glennsweeney]

@natj - works fine for me. I just used a naming convention from past (non-julia) projects. I'll remove the prefixes, and suffix _data on the end of the data files instead, if that sounds better. Also happy to move those functions back around; most of those changes were guesswork.

@m-lohmann - I was planning on at least writing DIN99d, as it seems to be most adopted. I'm also planning on moving in my set of color difference equations.

All: If we are going to start accumulating more color science / colorimetry data and databases; do we need a more structured way rather than just .jl files?

[glennsweeney]

@natj - I put all of your suggested changes so far in at https://github.com/glenn-sweeney/Color.jl/tree/refactor.

[natj]

Good catch @m-lohmann I corrected the name to be Bezier in my pull request #19

@glenn-sweeney Nice job! Now we'll have to wait what others think about this too...

[m-lohmann]

@glenn-sweeney - Concerning colorimetry data: I think both ways would be possible. We could store measurement spectra as matrices, similar to what is already done for the standard observer file cie_color_matching.jl. What other file types would or could be desirable? Csv?

[dcjones]

@glenn-sweeney This looks good, I like how it turned out.

I don't know that storing data in external (non-jl) files has any advantage other than potentially letting us load it lazily on use. I suspect that's not worth the trouble unless it ends up being quite a lot of data.

Should cie_color_match live in cie_data.jl or somewhere rather than utilities.jl? It seems slightly out of place there. If the plan is to add more datasets, we could just add a new file for functions like this used to access data.

[glennsweeney]

@m-lohmann - I think for observer functions and similar can stay in .jl files. However, you mentioned a database of Macbeth spectra; I think that if databases like that (as opposed to standards-based references) begin to be incorporated, moving them out into dedicated files may be useful. I'm also not sure if that is the intent of the module or not; others who have been working on this package longer can speak to that better than me. Otherwise, I think that using jl files will be good for current purposes.

@dcjones - I stuck it in utilities.jl because i was trying to keep cie_data.jl function-free. However, if we view it as a frontend to the data, I guess it makes sense to move it back there.

I think all of this spawns an interesting question; is the Color module going to be responsible for providing databases to users as well as functionality? Right now the data here is really pretty mandatory for basic color computation, but it sounds like there is interest in adding more. Are there opinions on this from the library maintainers?

[m-lohmann]

@glenn-sweeney - Yes, I think storing them in .jl files makes more sense than using other file formats.

I think it is useful to have datasets of spectra, JND ellipses and so on available—at least the basics, if the whole Color.jl module is not just intended to be a simple sRGB to CIELAB transformation tool but rather a toolset for more serious color metrics calculations and research. I mean datasets like the MacAdam ellipses, the Luo, Cui and Rigg datasets for color space uniformity evaluations and such, with a comprehensive set of color spaces, color difference metrics, or even more sophisticated color appearance metrics like CIECAM02 and iCAM06.

[dcjones]

Having written the initial version of Color.jl, my only ambition was to provide basic colorspace transformations and parsing useful for plotting. But I think it's terrific that it's growing into something much more than that. It's becoming something special; few, maybe no, languages have an equivalent library. So I am all for expanding it contain colorimetry data and more sophisticated metrics as long as the basic functionality isn't compromised.

[glennsweeney]

That's a really great direction to go I think.

As for basic functionality, I think that as more sophistication is added the underlying core is going to have to be generalized (RGB in my opinion could be renamed sRGB and a general RGB function written over it), but with proper design and documentation this shouldn't pose a problem down the road.

But all of this is a little bit past the refactor. For now, I'm going to move cie_color_match for a better home, and then when people are happy, I'll put in a pull request.

[m-lohmann]

@dcjones - I think there is no need for changing the code such that basic functionality would be a pain to use. So it is no problem to just convert colors between colorspaces or use the more sophisticated things like those I mentioned. Most of the latter is built on the basic things anyway or does not have any influence on the already existing code.

@glenn-sweeney - That sounds great. I’m waiting for it. And yes, preparation of RGB for more general use in terms of (device) color spaces like sRGB, AdobeRGB, CIERGB etc. should not be a problem at all. The difference is pretty much just using different conversion matrices tor each of these color spaces.

[timholy]

One small point is to consider the impact on load times. If the new functionality makes it many times slower to use, it could be an argument for having a "basic" package and an "advanced" package.

That said, we can already precompile packages, so this just isn't the concern it used to be. And hopefully usage of precompilation will get better over time. So overall I'm happy to have these very exciting extensions staying inside Color.jl, if that's what makes the most sense.

[dcjones]

Fixed with #24.