Skip to content

Conversation

@cvanelteren
Copy link
Collaborator

Fixes #86

Copy link
Collaborator

@beckermr beckermr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. This needs a test as well.

@cvanelteren cvanelteren requested a review from beckermr February 20, 2025 14:40
@cvanelteren
Copy link
Collaborator Author

Pinged a little too early -- it was a bit of a rabbit hole I went into but I think this should be correct.

@cvanelteren
Copy link
Collaborator Author

Some tests fail -- particularly on the colormaps. Judging from the actual differences they are minor. I am not exactly sure why they are different. Perhaps @beckermr can spot a mistake with some fresh eyes. Can confirm that the plot on #86 looks at it should now.

@beckermr
Copy link
Collaborator

My guess is that the Cycle object itself is an iterate so you need to move that case statement above the one with Iterable or add a new case statement after the one that catches Cycle objects.

@cvanelteren
Copy link
Collaborator Author

Good catch. That fixed it. Results look good now. The failure relates to a missing test and a fixed error.

@cvanelteren
Copy link
Collaborator Author

image
image

@beckermr
Copy link
Collaborator

That helped a ton. There is one failure in the box plots that needs investigating.

@beckermr
Copy link
Collaborator

Ah so the new boxplot plot is the correct one?

Copy link
Collaborator

@beckermr beckermr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming the new boxplot is correct, Lgtm!

@cvanelteren
Copy link
Collaborator Author

New one is correct I think. Thanks will push some styling and then merge it.

@cvanelteren cvanelteren merged commit f21a814 into Ultraplot:main Feb 20, 2025
9 of 12 checks passed
@cvanelteren cvanelteren deleted the allow-tuple-parse-cycle branch February 20, 2025 16:48
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

Successfully merging this pull request may close these issues.

Cycle object not parsed correctly as tuple

2 participants