Skip to content

Conversation

frutti93
Copy link
Contributor

As of now the color of axis labels is not considered in the tikz output. This small change fixes this.

@frutti93 frutti93 changed the title Update _axes.py transfer-axis-label-colors Sep 17, 2021
@frutti93 frutti93 changed the title transfer-axis-label-colors Transfer axis label colors Sep 17, 2021
@frutti93
Copy link
Contributor Author

frutti93 commented Sep 30, 2021

I am going to fix the tests next week ;)

@nschloe
Copy link
Owner

nschloe commented Oct 12, 2021

What are these dockerfiles?

@frutti93
Copy link
Contributor Author

Sorry, my mistake. I develop in containers and usually keep the dockerfiles in my gitignore. Obviously forgot it this time. I just removed them.

@nschloe
Copy link
Owner

nschloe commented Oct 12, 2021

There is another weird file.

@frutti93
Copy link
Contributor Author

frutti93 commented Oct 13, 2021

Removed. But this is nothing that is introduced by my changes. This file is reproducibly generated by running tox, even on main. Thats why i left it there. I added it to the gitignore so no one else commits it by mistake.

@nschloe
Copy link
Owner

nschloe commented Oct 13, 2021

... and another one.

@frutti93
Copy link
Contributor Author

... and again, these file are generated by running the unittests in general, they have nothing to do with this PR.

@nschloe
Copy link
Owner

nschloe commented Oct 13, 2021

What's the label color by default?

@frutti93
Copy link
Contributor Author

Tikz and matplotlib use 'black', however, for matplotlib it depends on the chosen plot style.

@nschloe
Copy link
Owner

nschloe commented Oct 13, 2021

I understand. I would probably lean towards "explicit is better than implicit" nowadays, too, but tikzplotlib has made a choice years ago to not include code that's default anyway. The idea was to keep the output TeX file reasonably lean. I would hence suggest to only include the color spec if it's not black.

@frutti93 frutti93 requested a review from nschloe October 18, 2021 06:50
@nschloe
Copy link
Owner

nschloe commented Oct 18, 2021

LGTM now. Thanks!

@nschloe nschloe merged commit b564b88 into nschloe:main Oct 18, 2021
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.

2 participants