Skip to content

Refactor and add opinionated_min style with example#2

Open
saforem2 wants to merge 47 commits intoMNoichl:masterfrom
saforem2:master
Open

Refactor and add opinionated_min style with example#2
saforem2 wants to merge 47 commits intoMNoichl:masterfrom
saforem2:master

Conversation

@saforem2
Copy link

@saforem2 saforem2 commented Sep 9, 2023

Changes:

  • Move src/opinionated/data/*.mplstyle to stylefiles/*.mplstyle
  • Remove src/opinionated/fonts/* from git
  • Modified src/opinionated/{__init__.py,core.py} to:
    • Automatically download any missing fonts on import opinionated
    • These will be downloaded into fonts/ in the project root
  • Remove {setup.py,setup.cfg}, only use pyproject.toml
  • Updated .gitignore
  • Added opinionated_min example in README and stylelib/opinionated_min.mplstyle

@saforem2
Copy link
Author

saforem2 commented Sep 9, 2023

sorry, just your comment about building / publishing!

You can use hatch:

(I like the dynamic versioning 🤷🏻‍♂️ )

$ hatch version
0.0.1

$ hatch build
[sdist]
dist/opinionated-0.0.1.tar.gz

[wheel]
dist/opinionated-0.0.1-py3-none-any.whl

8.75s user 1.16s system 78% cpu 12.589s total
$ hatch publish -r test
Enter your username: saforem2
Enter your credentials:
dist/opinionated-0.0.1.tar.gz ... success
dist/opinionated-0.0.1-py3-none-any.whl ... success

Honestly I was initially reluctant about the reorg / big changes, but it is called opinionated so I took some liberties 😂

@MNoichl
Copy link
Owner

MNoichl commented Sep 10, 2023

Hey, very nice!
No, I think I will like the reorg! It might take a little time for me to look over everything though, because I'm busy right now, and I want to be very careful not to break things, now that people have started using it.

What's the reasoning behind the opinionated_minimal style? If the only difference is that it uses IBM Plex Sans I would rename it to opinionated_ps to keep things consistent and to keep in line with the allusion to hrbrthemes, if that's fine with you.

@saforem2
Copy link
Author

yeah that makes sense.

No, I think I will like the reorg! It might take a little time for me to look over everything though, because I'm busy right now, and I want to > be very careful not to break things, now that people have started using it.

yeah no worries! I have a few other ideas for changes to things that I'll keep playing around with
(/ would be happy to discuss further if you'd be interested ?? )

One thing I was thinking about:

It seems the only differences (currently) between the opinionated_*.mplstyle's is the font choice, right?

One thing we could do would be to split up the {font,style} components of the config so that you could do something like:

from opinionated import use_font, use_style
use_font('IBM Plex Sans')  # 'Roboto Condensed', ...
use_style('transparent')   # 'default', 'light', 'dark', 'bright', ...

so that you could define / configure these components individually as needed.

@saforem2
Copy link
Author

Also, I've updated started updating this build_example_plots.ipynb

currently a WIP and I need to add back in what you had in there originally, but just thought I'd point it out

@saforem2
Copy link
Author

I also noticed you had this docs/ that I'm assuming was intended to host the documentation / public facing website.

I've recently been independently working on simplifying this process for a bunch of my other projects / repos.
I've made a new branch in my fork with a simple example of how this could be done

if that's something you'd be interested in 🤷🏻‍♂️

@MNoichl
Copy link
Owner

MNoichl commented Sep 12, 2023

Hey, thank you for all your work! Should I convert this into a branch? Maybe that makes it easier to go forward.

would be happy to discuss further if you'd be interested

Sure, let's talk! I'm also happy to talk via Zoom or something. Maybe that's quicker.

One thing we could do would be to split up the `{font,style}` components of the config

Hmm. I don't think I want to encourage that to be honest. My idea with the opinionated package is that it should not only make it easy to get plots with better fonts but also discourage people from making bad choices (that's the opinionated part).

So I gave some thought about which fonts to include, and I'm actually thinking more about kicking out some of the styles that I include at the moment – Montserrat will not look good in most plots, because it's too wide, and although I love the font, a similar thing is true for Space Grotesk – than adding or encouraging other fonts. I was also thinking about finetuning the designs (margins, sizes etc.) for individual fonts in the future, haven't gotten around to it though.

To me it seems that the font/style thing, if set as a default encourages experimentation, while the current code (download & set font) enables it, but doesn't force the issue. Of course, I see why, from a programmer's perspective of code-reusability, your solution would be quite elegant.

Also, I'm actually not a big fan of a dark mode for plots. I just don't think I see them used effectively that often, even on dark websites. So I somewhat consciously did not include them. But as people apparently will want to use them, maybe we should have one well-designed dark mode as a feature. Maybe similar to the hrbrthemes ft-theme. This could be set by doing something like opinionated_rc_d to the style - what do you think? Or we could distinguish between web and article plots, with the latter having smaller fonts, closer labels, and one well thought out serif style...

I do like plots with greyish-beige backgrounds, and am planning to do a clone of opinionated, which allows you to make pastiches of historic datavis, with works by Nightingale, Playfair, Minard, Neurath, etc. but I feel that's best left to a different project.

Also, I've updated started updating this [`build_example_plots.ipynb`](https://github.com/saforem2/opinionated/blob/master/src/opinionated/notebooks/build_example_plots.ipynb)

I actually meant to delete these notebooks, as I have stopped using them. Probably it would be better to have a script generate the plots than a notebook anyway (?).

I also noticed you had this [`docs/`](https://github.com/MNoichl/opinionated/tree/master/docs) that I'm assuming was intended to host the documentation / public facing website.

That does look cool! I was thinking of doing some sphinx docs, but then I felt that the project is small enough that we don't really need docs (?)

@saforem2
Copy link
Author

yeah definitely, I'm wrapped up today and tomorrow but might have some free time Friday afternoon if you're available and still want to hop on a zoom call to discuss further

Hmm. I don't think I want to encourage that to be honest. My idea with the opinionated package is that it should not only make it easy to get plots with better fonts but also discourage people from making bad choices (that's the opinionated part).

fair enough, honestly I was interpreting it as the users being the opinionated ones, i.e. allow them to easily customize / modify existing themes (with sensible opinionated defaults), which I guess is really a different end goal than you might've had in mind.

Also, I'm actually not a big fan of a dark mode for plots. I just don't think I see them used effectively that often, even on dark websites. So I somewhat consciously did not include them.

But as people apparently will want to use them, maybe we should have one well-designed dark mode as a feature. Maybe similar to the hrbrthemes ft-theme. This could be set by doing something like opinionated_rc_d to the style - what do you think?

again, completely fair point (and I know a lot of people don't particularly like dark themes in general)

An easy fix, in my opinion, is to use a single style with:

  • transparent background
  • grey (#838383) text (so the image is legible on both dark and light backgrounds)
  • .svg for vector graphics (+ ability to modify directly)

which is what I was going for in the opinionated_minimal.mplstyle, and which I (again, personally) like to use as a starting template when I make my own figures.

e.g.

but honestly if you have different goals in mind for this project (and/or would prefer to maintain that vision and keep things as is), I can abandon this pull request and just create a new repo / project and mention that it is an offshoot of MNoichl/opinionated or something??

@saforem2
Copy link
Author

Should I convert this into a branch? Maybe that makes it easier to go forward.

yeah, if that would be easier for you that's totally fine with me.

I've done some additional poking around and I'm almost positive everything from the original implementation still works as is, with some additional features added (coming from the TODOs in the original project, e.g.

  • automatically checking if font exists in opinionated.FONTS_DIR before trying to download again

  • remove font files from GitHub repository, and download any missing fonts automatically at runtime

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