| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
Hi, thanks for following up. I'm not sure if your tests are in draft form, but they're currently not testing anything because your reference and actual figures are using the same code to make the same image. You may need a mix of comparison and equals_tests here https://matplotlib.org/devdocs/devel/testing.html#writing-an-image-comparison-test
Sorry, something went wrong.
|
Hi @Vashesh08 , it looks like you pulled in a number of unrelated commits into your branch. You will probably need to rebase your branch to leave only your commits. If you run into trouble and need help, let us know. |
Sorry, something went wrong.
|
@melissawm I think I removed the unrelated commits. Thanks for the docs! |
Sorry, something went wrong.
There was a problem hiding this comment.
Hi,
I'm really sorry I dropped the ball on finishing this up. If you've got the time, my major question/concern/thing I should have flagged earlier (sorry :wince:) is that the hatchcolor shouldn't be set inside edgecolor. I think seperating it out so that hatchcolor is only set inside set_hatchcolor, even when set to the edge color, should clean up some of the logic chains happening now and will hopefully get rid of the need for a second set_hatch_color variable.
Sorry, something went wrong.
|
I'm extremely sorry for the delay in responding. I missed out on your response. The primary concern I had was how should I tackle the hatchcolor when both hatchcolor and edgecolor values both are specified. Since these values are updating using _internal_update method, which makes the order of these parameters important. In order to tackle this, I had to introduced a class variable self.set_hatch_color and local function variables set_hatchcolor_from_edgecolor. the hatchcolor shouldn't be set inside edgecolor I will be pushing these changes. Lmk if you still think there are room for any more improvements. |
Sorry, something went wrong.
There was a problem hiding this comment.
I might be missing something, but I don't see why this isn't just a fallback chain. Also sorry it took so long to get back to you.
Sorry, something went wrong.
|
hi, we discussed this on the call this week b/c I was worried and @ksunden and @QuLogic both suggested that there's a way to inherit the rcparam from edgecolor so that you can reduce the fallback logic and that legend.facecolor already tracks legend.edgecolor and you can maybe use that as a model. What that means is it solves the black default or intentional problem so
Also thanks for your patience and perseverance on this. |
Sorry, something went wrong.
|
@Vashesh08 are you coming back to this? Or should I give it a go |
Sorry, something went wrong.
|
@Impaler343 |
Sorry, something went wrong.
|
gonna close this since #28104 supercedes this |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR summary
Fixes #26074
Continued From #26683
PR checklist