Visual tests - correct code to pass and remove xfails

[CyclingNinja]

No description provided.

[astrofrog]

pre-commit.ci autofix

[astrofrog]

This looks good to me, though it looks like there might be a genuine failure in the line style test. Once this is fixed, the hashes for the figures should be updated for the two tests which will allow the CI to pass.

[CyclingNinja]

pre-commit.ci autofix

[astrofrog]

Looking good, I think you just need to update the hashes now!

[codecov]

Codecov Report

Attention: Patch coverage is 18.46154% with 53 lines in your changes missing coverage. Please review.

Project coverage is 84.41%. Comparing base (f997e7d) to head (cdcd961).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
glue_jupyter/bqplot/scatter/tests/test_visual.py	0.00%	29 Missing ⚠️
glue_jupyter/bqplot/scatter/layer_artist.py	29.41%	24 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #479      +/-   ##
==========================================
- Coverage   85.07%   84.41%   -0.67%     
==========================================
  Files          91       91              
  Lines        5407     5447      +40     
==========================================
- Hits         4600     4598       -2     
- Misses        807      849      +42     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dhomeier]

Seems test_remove is failing now (due to changed number of tests?)
And the two failing image comparisons look like it’s just a change in colourmaps, so just update the hashes again?

[dhomeier]

I don't understand the logic behind renaming the property to color here and in just a couple other instances below – colors is still present in various other instances, so this should at least remain consistent. Looking further at the existing instances e.g. of scatter_mark.color, the latter I think is generally a single colour, while colors always is a list, even if only a single element one. May need to review #363 where those were introduced though.
But I am wondering if this is related to the colour mismatches in the ui tests.

[CyclingNinja]

Okay, having refreshed my brain, the color vs colors comes from the altering between lines_gl and lines. What eventually becomes self.line_mark_gl and self.line_mark see 337 and 338

It alternates between the two in order to handle the switch to non gl linestyle. Is it readable, not so much, but it does work

[dhomeier]

Needs some explanation in comments or here in the PR then; from the above it’s anything but clear how this is related to a lines vs. lines_gl alternation. And here in __init__ at least self.vector_lines.colors is no longer set at all (or left at default, if there is one).
With the bunch of tests still failing it’s hard to tell if it does work; both the colour mismatches in CircleCI and warnings like BROWSER LOG: no color set in ui-tests still look suspicious.

[dhomeier]

I don't exactly understand the changed test_visual_linestyle, adding two more layers and then removing them again without further testing? Looks like the outcome should be the same as with the original 3 layers, but the failure indicates some expected number (of layers?) has changed.
The colors issue definitely needs to be sorted out.
py314 error on scikit-image under Windows is unrelated, though I have no idea why it would install the py313 wheel last week and now refuses to (but apparently still uses it on Linux and macOS!). But a new rc for 0.26 just came out today; dunno if that's related, but hopefully there will be a version with proper py314 support soon.