elixir-image/image

Image.compose dialyzer issue

Closed this issue ยท 8 comments

tielur commented

It looks like theres a spec issue with the Image.compose! and Image.compose function when you give it a list of images with options

Example:

small_kip = Image.open!("test/images/Kip_small.jpg")

texts = [
  {Image.Text.text!("hello", text_fill_color: :darkslategray, padding: 10),
   x: :center, y: :middle},
  {Image.Text.text!("world", text_fill_color: :darkslategray, padding: 10), dy: 12}
]

Image.compose!(small_kip, texts)
The function call will not succeed.

Image.compose!(
 _small_kip :: %Vix.Vips.Image{:ref => reference()},
 _texts :: [
   {%Vix.Vips.Image{:ref => reference()},
    [{:dy, 12} | {:x, :center} | {:y, :middle}, ...]},
   ...
 ]
)

will never return since the 2nd arguments differ
from the success typing arguments:

(
 %Vix.Vips.Image{:ref => reference()},
 [%Vix.Vips.Image{:ref => reference()}, ...] | %Vix.Vips.Image{:ref => reference()}
)
tielur commented

If you checkout main branch of https://github.com/tielur/replace_with_test and run mix dialyzer you'll see the above dialyzer warning

Thanks for the report. Ah, my old "friend" dialyzer. I'll get into this now. Just to confirm - the code works as expected its just a dialyzer/spec issue?

tielur commented

Thanks for the report. Ah, my old "friend" dialyzer. I'll get into this now. Just to confirm - the code works as expected its just a dialyzer/spec issue?

Correct, code works great! Just a spec issue
iTerm2 jT8PfM

๐Ÿ˜†

tielur commented

@kipcole9 would you be opposed to me opening a PR to run dialyzer in CI? I have some experience with setting it up and caching the PLT's so CI remains speedy

I'm slowly working my way through moving all my libs to GitHub actions - I'll set that up now for this lib. I appreciate the offer though, much appreciated.

BTW, I run dialyzer on everything. But of course if there is no function call for a certain combination then it won't always pick up issues. I've added your example above to the repo now so dialyzer will check it and I'll resolve the issue over the next coffee.

This is going to take a little while - the usual ripple effect. I'll have it sorted out today though (in the next 12 hours).

tielur commented

This is going to take a little while - the usual ripple effect. I'll have it sorted out today though (in the next 12 hours).

Sounds good! I'm ignoring the warning for now on my side, nice that dialyzer allows that for upstream things like this

Appreciate all your responses and great work

Per your suggestion I've configured Github Actions to run tests and dialyzer on push. I know its not a work of art so if your offer to help improve it still stands, PRs will always be welcome.

I'm omitting some tests in CI for now:

  • Text tests because the fonts on Mac and Linux are different so all the tests fail. I need to find an acceptably similar font on both platforms to standardise tests
  • Some avatar tests since they use text
  • Some video tests since there is no camera in CI

Lastly, I'm building libvips from source in CI. This is probably overkills but I did want to validate end-to-end from source the whole system. The precompiled version of the NIF and libvips from vix doesn't include some plugins that tests depend on like ffmpeg. So I'll probably go with whatever apt-get installs for libvips eventually.