[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 11 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 11 months ago
gph82 force-pushed fix_colors from 7909b42277 to 2dde879a24 11 months ago
gph82 closed this pull request 11 months ago
tfardet added the
behavior-change
label 11 months ago
tfardet approved these changes 11 months ago
tfardet left a comment
Owner

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 ;)

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 11 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 11 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.