Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logo-handling class and remote fetching of Ouranos logos #119

Merged
merged 23 commits into from
Oct 17, 2023

Conversation

Zeitsperre
Copy link
Contributor

@Zeitsperre Zeitsperre commented Oct 5, 2023

Pull Request Checklist:

What kind of change does this PR introduce?

  • Adds a Logos class for setting and installing logos to be used
  • Changes the behaviour of plot_logo() to accept a pathlib.Path to a location on disk or a str identifying an installed logo.
  • Added some documentation in usage.rst and some markdown cells in figanos_docs.ipynb better detailing how the logos are installed and used.
  • Added an image rescaling function for resizing logos or images using scikit-image.

Does this PR introduce a breaking change?

Yes. The Ouranos logos are not installed by default and will be removed from the repository and fetched on request from the Ouranosinc/.github repo. The Figanos logo is installed by default now.

We also now depend on scikit-image for image resizing.

Other information:

Some things that are missing so far:

  • Integration of the Logos class into the matplotlib functions.
  • Scaling of the logos according to the size of the output figures (Is this implemented? Yes)
    • Scaling is related to the size of the PNGs
    • Alrighty then, we have an entirely new system in place.
  • Documentation detailing how this works

@Zeitsperre Zeitsperre added the enhancement New feature or request label Oct 5, 2023
@Zeitsperre Zeitsperre self-assigned this Oct 5, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@Zeitsperre Zeitsperre marked this pull request as ready for review October 6, 2023 20:29
docs/usage.rst Outdated Show resolved Hide resolved
figanos/matplotlib/utils.py Outdated Show resolved Hide resolved
docs/usage.rst Outdated Show resolved Hide resolved
docs/notebooks/figanos_docs.ipynb Outdated Show resolved Hide resolved
docs/notebooks/figanos_docs.ipynb Outdated Show resolved Hide resolved
docs/notebooks/figanos_docs.ipynb Outdated Show resolved Hide resolved
docs/usage.rst Outdated Show resolved Hide resolved
@Zeitsperre
Copy link
Contributor Author

This PR is complete IMO. The notebook shows failures since I killed one cell when running on my local machine. My other PR (#121) tries to fix this up in sphinx/RTD.

@review-notebook-app
Copy link

review-notebook-app bot commented Oct 10, 2023

View / edit / reply to this conversation on ReviewNB

juliettelavoie commented on 2023-10-10T20:57:43Z
----------------------------------------------------------------

Can we set 'ouranos_logo_vertical_couleur' as the default when installing it ?


Zeitsperre commented on 2023-10-10T21:51:29Z
----------------------------------------------------------------

This is now the case.

Copy link
Contributor Author

This is now the case.


View entire conversation on ReviewNB

Copy link
Collaborator

@sarahclaude sarahclaude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ouranos logo installation works great and it's nice to have all the other options (black, white, etc).
Using pathlib.Path(path in string) doesn't seems to work for me. Should I use pathlib another way?
Also, the error mesage due to the path not working tells me no logo are installed but this is not really the case as I am trying to show another logo than the installed one.

HISTORY.rst Outdated Show resolved Hide resolved
figanos/matplotlib/utils.py Show resolved Hide resolved
…to dependencies, support more intuitive logo-handling, add examples
@Zeitsperre
Copy link
Contributor Author

We now support SVGs:
image

Copy link
Collaborator

@sarahclaude sarahclaude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L'ajout de logo avec Pathlib.Path fonctionne et le logo de défaut installé aussi!
Je suppose que le "blurry" du logo d'Ouranos serait corrigé avec le fichier .svg envoyé par Sarah (plutôt que l'utilisation de .png)?

@Zeitsperre Zeitsperre merged commit e73be4d into main Oct 17, 2023
7 checks passed
@Zeitsperre Zeitsperre deleted the explicit-logo-download branch October 17, 2023 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Figanos should rescale logos/images to fit the intended purpose Standardized folders for logos and badges
3 participants