Arcadia-Science / arcadia-pycolor

Python package for styling plots according to Arcadia's style guide
MIT License
4 stars 0 forks source link

Adding matplotlib-specific styling functionality #14

Closed mezarque closed 4 months ago

mezarque commented 4 months ago

PR checklist

Overview

This PR adds in automatic styling functionality for matplotlib, giving users the ability to more easily perform style modifications to plots. The modifications occur primarily through two parts of the API:

In addition to those major functions, there's some auxiliary functionality to help figures conform to fixed size guidelines, which are provided via existing templates in our Adobe Creative Cloud library.

Drive-by (is this the proper usage of this? I can't find references online):

There's a lot going on here, and I'm sure we'll have a lively discussion about things like naming, what to expose to users, organization, namespaces, etc. Looking forward to lots of feedback!

Testing

I confirmed that things work as expected by testing the functionality in usage_example.ipynb, which goes into some detail about what all is going on and how users might use the various functions.

Halp

CI is failing right now; it seems that impresources is not finding the arcadia_2024.mplstyle file and is returning None. I am not sure how to resolve this, and would love some help figuring out how to get the CI to work properly; this error does not show up when I run pytest . on my local clone.

TODO

These are things that I'd like to figure out, but haven't yet found an elegant way to implement.

review-notebook-app[bot] commented 4 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

mezarque commented 4 months ago

I haven't fully reviewed the mpl changes, but I made a couple suggestions to start a discussion. I am not sure how much research you've done around available tools, but I would check these two tools as potential inspiration: https://github.com/raybuhr/pyplot-themes and https://github.com/lgienapp/aquarel

Thanks for pointing me to these! I do like how aquarel uses with to specify a context for plotting; this could save users from having to have to call autostyle at the end of plots. I think both packages do some of the things we need to accomplish, but not all. Configurability/ usability is also something we want to consider, since I think users may not know what changes they want to make in advance of generating a plot. Let's explore these different approaches through some user testing.

mezarque commented 4 months ago

I think I addressed most of the comments above - thanks for all the feedback! My main concern with the renamings is that all of the functions are now quite verbose, which feels clunky / ugly to use. But maybe that's just the sacrifice we have to make for clarity.

Should I write tests for the functionality in this PR, or wait for another PR?

mezarque commented 4 months ago

What happened to the tests? They seem to have disappeared from the CI workflow.

I'm not sure where you're looking, but at least in my view of the PR, the tests are still running (and if you look at the Actions tab, it seems like they're working still?)

I addressed the remaining comments; lmk if you have any further Qs!