fix tests from modified `get_element_instances`

[giovp]

folllow https://github.com/scverse/spatialdata/pull/621 and should be merged after that.

I modified couple of plots that I think it made sense, but for two, specifically the NotebookTransformation ones for rotation and affine I did not, you can see below the new version (left) v. old version (right)

affine

rotation

you can see that the only thing that really changes is the background color, I wonder if it is a matplolib version problem, as I don't think spatialdata#621 impacts that. Any idea @melonora @timtreis ?

[melonora]

yeah this is a known issue, matplotlib is producing different results dependent on both version and platform.

[giovp]

ok so the tests should pass for those two 🤞

[melonora]

hmm this label_categorical color was wrong and is still wrong. Given that it was broken I am ok with this PR, but we need to fix it. What is indicated as being C is actually background label. @timtreis I vaguely remember you workin on a fix for this. Am I correct?

[giovp]

I'm afraid the tests failing are the ones of the figures I have added, I wonder if the reason is precisely this version differing behaviour. Increase tolerance ? or someone has an ubuntu machine to recreate figures?

[melonora]

Increasing tolerance will not help as this does not account for large difference in colors which happens with different color for the labels or background.

[timtreis]

Yeah, no idea why this is happening. Noticed it in https://github.com/scverse/spatialdata-plot/issues/259 originally but I still have no idea what's causing it. Local to me it looks fine. For this case, I'm just using the image of the runner itself.

[giovp]

Good point, uploaded the new ones from the runner artifacts

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.76%. Comparing base (d43d3e7) to head (7935a44).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #287   +/-   ##
=======================================
  Coverage   83.76%   83.76%           
=======================================
  Files           8        8           
  Lines        1694     1694           
=======================================
  Hits         1419     1419           
  Misses        275      275           

[giovp]

wtf how can 3.9 and 3.10 not be consistent?

[melonora]

Because matplotlib. Had this before, in these cases for now if this happens we accept the PR

[giovp]

mmh but this is a bit suspicious, like the results seems to put labels with different orders, despite everything being generated by RNG. In the last commit, I took artefacts from 3.9 and copied it to 3.10 and still now both fails

[melonora]

@timtreis are you still planning to pick this up? Otherwise I can.

[timtreis]

I think I tightened the GH actions enough so that 3.9 and 3.10 are now consistent. At least I didn't run into inconsistencies in the few last PRs. We still get the black bg on some of the raccoons, only on the runner though.

Can this be closed?

[LucaMarconato]

I checked the code and the one from Giovanni is correct. The max of the labels is 5, but the labels are non-contiguous, so using len(get_element_instances()) is the way to go.

[LucaMarconato]

As pointed out by @melonora, the plots from this commit https://github.com/scverse/spatialdata-plot/pull/287/commits/42a4ee697644333fd2f82b8c2629d646cfabf778 were still wrong as the background should have been black and not colored.

The reason was this line here: https://github.com/scverse/spatialdata-plot/blob/42a4ee697644333fd2f82b8c2629d646cfabf778/tests/pl/test_render_labels.py#L123 which was replacing the new correct value for the instance_id column, with a wrong one, starting from zero.

I have corrected this, verified the consistency with napari-spatialdata and regenerated the ground-truth plot. Now it's ready to merge.