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

avg_fun and transform #22

Open
dylanbeaudette opened this issue Dec 21, 2022 · 3 comments
Open

avg_fun and transform #22

dylanbeaudette opened this issue Dec 21, 2022 · 3 comments

Comments

@dylanbeaudette
Copy link

dylanbeaudette commented Dec 21, 2022

Thanks for this great package! There are a lot of interesting applications in soil science.

Is avg_fun applied to the original grid values, or values after transform is applied? I'm thinking about the application to RGB images, where computation of supercells and aggregation makes more sense in CIELAB vs. sRGB.

@Nowosad
Copy link
Owner

Nowosad commented Dec 22, 2022

Hi Dylan -- thanks for the kind words. I look forward to seeing {supercells} be applied in soil science.

Regarding your question:

  1. The supercells() function works as follows (when transform is used): first, the transformation is applied, then the supercells algorithm is run (with the given avg_fun), and afterward, a reverse transformation is performed on the obtained results.
  2. Frankly, I am not entirely convinced that the transform argument should even exist. It is a quick shortcut, but at the same time, users could just process their input first (from RGB to CIELAB) and transform the output back to RGB).

Let me know what you think about this argument (and if you have any more questions).

@dylanbeaudette
Copy link
Author

Hi Jakub,

Thank you for the clarification. If you choose to keep the transform argument then that text might be really helpful in the documentation.

For point number 2. This is something that I struggle with as well: balancing convenience / generalization of R functions. The current transform-to-LAB option is really important, given that supercells will be applied to sRGB image stacks and that the sRGB--CIELAB transform is critical to distances computed from color data. The forward / reverse transform isn't hard, but does require a little bit of coordination, accounting for NA (depending on the conversion function), and things like reference illuminants.

A couple of ideas here:

  • if the transform argument is only for sRGB--CIELAB--sRGB transforms, then rename the argument to toLAB or CIELAB as logical argument.
  • implement the transformation using grDevices::convertColor() or farver::convert_colour() vs. manually

@Nowosad
Copy link
Owner

Nowosad commented Jan 7, 2023

Hi @dylanbeaudette -- as for now, I just updated the docs (033bbdd).

Regarding point 2: I am still waiting for someone to give me another conversion that could be useful in transform. [On a side note: I can imagine, for example, accepting a list of two functions there, where the first function transforms the data before creating supercells and the second one do the reverse transformation...]

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

2 participants