[chord_diagram.chord_diagram] optarg 'order' now also applies to the fragment colors #31

Closed
gph82 wants to merge 1 commits from gph82:fix_colors into main
gph82 commented 6 months ago
Collaborator

Here we go again ;)

This one isn't urgent.

It feels like 'order' should also reorder fragment colors, specially if it reorders fragment names, right?

names=["A","B","C"]
colors=["red","green","blue"]

When passing the above arguments, the user probably wants "A"->"red", "B"->"green", "C"->"blue" regardless of order being [0,1,2], [2,1,0] or whatever.

E.g., when trying different orders, you want a each arch conserving color accross trials, to be able to "follow" them visually, right?

PS: I accidentally (pushed directly from terminal) created the branch 'fix_colors' in your original repo instead of my fork. I deleted it already, I think I should never push to yours. That was an accident, sorry!

Here we go again ;) This one isn't urgent. It feels like 'order' should also reorder fragment colors, specially if it reorders fragment names, right? ``` names=["A","B","C"] colors=["red","green","blue"] ``` When passing the above arguments, the user probably wants "A"->"red", "B"->"green", "C"->"blue" regardless of order being [0,1,2], [2,1,0] or whatever. E.g., when trying different orders, you want a each arch conserving color accross trials, to be able to "follow" them visually, right? PS: I accidentally (pushed directly from terminal) created the branch 'fix_colors' in your original repo instead of my fork. I deleted it already, I think I should never push to yours. That was an accident, sorry!
gph82 added 1 commit 6 months ago
gph82 force-pushed fix_colors from 7909b42277 to 2dde879a24 6 months ago
gph82 closed this pull request 6 months ago
tfardet added the
behavior-change
label 6 months ago
tfardet approved these changes 6 months ago
tfardet left a comment

Indeed, that sounds perfectly reasonable.

PS: no problem regarding you pushing to my repo, the only thing no one should do is push to main and it is protected so no one can anyway ;)

tfardet added the
enhancement
label 6 months ago
Poster
Collaborator

cool, but can we merge the other PR (#32) that has a better wording?

cool, but can we merge the other PR (#32) that has a better wording?

Reviewers

tfardet approved these changes 6 months ago
This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Loading…
There is no content yet.