Get color based on name using Enum #52

bobleesj · 2024-08-19T21:29:23Z

Closes #40

Motivation

We want the user to choose a color.
We do not want the user to memorize the color name.

Proposed solution

from bg_mpl_stylesheets.colors import Colors

# Get hex color code for BG_BLUE
Colors.bg_blue.value  # Example: '#0B3C5D'
Colors.bg_blue.name  # Example: 'bg_blue'

# Get a list of all bg-style colors
Colors.get_bg_colors()

@sbillinge If you find this implementation useful, I will update README.md

src/bg_mpl_stylesheets/tests/test_colors.py

src/bg_mpl_stylesheets/colors.py

sbillinge

This looks good except I don't think we should update the cycler which needs to be pretty immutable or users may get unwanted behavior.

sbillinge · 2024-08-20T06:56:22Z

@bobleesj please can you add a news item and update docs with this? It is really great. Ideally we would need a plot in the docs that shows the colors with the names. This is also needed to fine-tune the names.

sbillinge

please see inline.

sbillinge · 2024-08-20T12:29:24Z

news/colorupdate.rst


 **Changed:**

-* change of two colors for higher contrast: ``#AB987A`` to ``#918770`` and ``#984B43`` to ``#6C5050``
+* Change of two colors for higher contrast: ``#AB987A`` to ``#918770`` and ``#984B43`` to ``#6C5050``


This makes it appear that colors have changed, but no colors should have changed.

No colors have changed. Just captialize the first alphabet from "c" to "C"

If no colors have changed, please delete these change from the news.

example/color_cycle.png

bobleesj

Please review my PR.

Updated doc with instruction
Updated the example image legends from hex code to color name

It appears that the CI is failing perhaps due to cache.

I checked from my own repo CI that it runs.

https://github.com/bobleesj/bg-mpl-stylesheets/actions/runs/10475957028

src/bg_mpl_stylesheets/colors.py

sbillinge

@bobleesj this is really great. I love what you did here. Please see my inline comments and we can move to merge this. I really like the new plot of the colors.

Thanks for uppercasing all the hex's too....

sbillinge · 2024-08-20T20:19:04Z

news/colorupdate.rst


 **Changed:**

-* change of two colors for higher contrast: ``#AB987A`` to ``#918770`` and ``#984B43`` to ``#6C5050``
+* Change of two colors for higher contrast: ``#AB987A`` to ``#918770`` and ``#984B43`` to ``#6C5050``


If no colors have changed, please delete these change from the news.

src/bg_mpl_stylesheets/colors.py

bobleesj

Please review @sbillinge

Again, CI passes from my forked repo:

https://github.com/bobleesj/bg-mpl-stylesheets/actions/runs/10479038124

properties lower_cased instead of lower-cased since it's a property.
news on color change removed.

README.rst

src/bg_mpl_stylesheets/colors.py

bobleesj · 2024-08-20T21:03:13Z

src/bg_mpl_stylesheets/tests/test_colors.py

+    assert hex == expected_hex
+
+
+@pytest.mark.parametrize(


Here, get_color_name(hex) could instead return a string of bg-color instead of bg_color but the user probably would be storing the color name string and use that to get the hex code (maybe)

Keeping all lower_cased would be a simple solution for all. @sbillinge Do you agree?

sbillinge

thanks, one last thing on the news.

sbillinge · 2024-08-20T21:41:39Z

news/colorupdate.rst

@@ -4,7 +4,6 @@

 **Changed:**

-* Change of two colors for higher contrast: ``#AB987A`` to ``#918770`` and ``#984B43`` to ``#6C5050``
 * Capitalized the hex color codes


I think this should be under fixed. Changed refers to changes that would affect the user.

I think this should be under fixed. Changed refers to changes that would affect the user.

Noted. Moved and please review.

sbillinge

thanks. one comment again...

news/colorupdate.rst

bobleesj

Please review!

news/colorupdate.rst

sbillinge · 2024-08-21T02:02:44Z

It looks like the problem is a lower case b in
'axes.prop_cycle': cycler('color', ['#0B3C5D', '#B82601', '#1C6B0A', '#328CC1', '#A8b6C1', '#D9B310', '#6C5050', '#76323F', '#626E60', '#918770', '#C09F80', '#B0B0B0FF']),

bobleesj · 2024-08-21T02:14:10Z

!! @sbillinge thanks

sbillinge

Few comments that could help the usability

sbillinge · 2024-08-21T02:19:15Z

src/bg_mpl_stylesheets/colors.py

+        for color in cls:
+            if color.value.lower() == hex_value.lower():
+                return color.name
+        raise ValueError("Unknown color name. Please check Colors enum for available colors.")


Would a non programmer user know how to do this? I think this is too jargony to be helpful.

How about this?

raise ValueError("You have entered an unknown color hex value." " Please check the src/bg_mpl_stylesheets/colors.py file for the available colors.")

You have entered an unknown color hex value -> reason for the error
Please check the src/bg_mpl_stylesheets/colors.py file for the available colors -> what to do fix to it

Yes, much better error message. Even better would be a method that lists the available colors maybe?

Wait, we already have that method, so could we mention that in the error message as an option? Look in the file or run that method?

README.rst

sbillinge

just a few more thoughts to make this more usable? Not sure what you think.

sbillinge · 2024-08-21T11:42:47Z

src/bg_mpl_stylesheets/colors.py

+    # Add more colors as needed
+
+    @classmethod
+    def get_color_name(cls, hex_value: str) -> str:


Do we actually need these. It seems that we are not making the best use of enums. For example, for usage, wouldn't something like this be cleaner?:

import bg_mpl_stylesheets.colors.Colors as bgcolors for color in bg.colors: ax.plot(x, y + offset * i, label=color.name, color=color.value, linestyle="-")

and if we want to address explicitly, something like

bgblue = bgcolors.bg_blue ax.plot(x, y + offset * i, label=bgblue.name, color=bgblue.value, linestyle="-")

I see that we would need the hex to name converter when using the cycler as a cycle unless we could figure out how to make the cycler elements the enum values.

class Colors(str, Enum): bg_blue = "#0B3C5D" bg_red = "#B82601" bg_green = "#1C6B0A" bg_light_blue = "#328CC1" bg_light_grey = "#A8B6C1" bg_yellow = "#D9B310" bg_brown = "#6C5050" bg_burgundy = "#76323F" bg_olive_green = "#626E60" bg_muted_olive = "#918770" bg_beige = "#C09F80" bg_grey = "#B0B0B0FF"

Right, this would be it based on your suggestion above.

The purpose of get_color_name was for testing below and changing the legends from hex to color names:

In the example code under example/color_cycles.py, we are testing the hard coded bg-style.

cycle = plt.rcParams["axes.prop_cycle"].by_key()["color"] # plot the color cycles and corresponding color codes. for i, hex in enumerate(cycle): color_name = Colors.get_color_name(hex) ax.plot(x, y + offset * i, label=color_name, color=hex, linestyle="-")

If do not have the get_color_name class method, then we do not have access to the color name since the hex a string.

If retrieving the color name from a string is not needed (legends for the the example code), then, we can remove get_color_name

Is there no way to get that figure as it is but without the get_color_name method but using the enum properties? That may be true, but something like replacing the hex hard code with the enum values could fix it? It is still "immutable" in the way that someone could extend the colors enum and it wouldn't change the cycler (which is what we seek for the immutability). We could also test it with hard coded hex's so if someone changes the hex of one of the used enums the test would break.

bobleesj · 2024-08-21T12:33:17Z

I will address the comments later in the afternoon and see how we can make this package useful.

sbillinge

please see comments. I have to hop on a flight. I will look at more later.

sbillinge · 2024-08-22T10:37:28Z

README.rst

+        bg_blue_hex = Colors.bg_blue.value
+
+        # Get color name
+        bg_blue_name = Colors.bg_blue.name


here we are teaching people somewhat ugly coding practices so I would remove these bg_blue_hex etc. variables.

We do want to let people know how to access the name and hex values, but let's do that with words, not with bad code....

The beauty of the enum is how easy and pythonic it is to access the names and values.

I am not totally sure why we don't use a dict, but it is ok

Yup value and name directly accessed in the example.

bobleesj · 2024-08-22T10:53:02Z

please see comments. I have to hop on a flight. I will look at more later.

Safe flight. I'll give this some thought and address them in the afternoon.

…lesheets into color-select

bobleesj

@sbillinge Please review!

bobleesj · 2024-08-24T21:04:57Z

example/color_cycles.py

-    ax.plot(x, y + offset * i, label="\\{}".format(c), color=c, linestyle="-")
-
+for i, hex in enumerate(cycle):
+    ax.plot(x, y + offset * i, label=Colors(hex).name, color=hex, linestyle="-")


Is there no way to get that figure as it is but without the get_color_name method but using the enum properties? That may be true, but something like replacing the hex hard code with the enum values could fix it? It is still "immutable" in the way that someone could extend the colors enum and it wouldn't change the cycler (which is what we seek for the immutability). We could also test it with hard coded hex's so if someone changes the hex of one of the used enums the test would break.

@sbillinge You are correct.

I found out that we do not need get_color_name. Python's enum provides Colors(hex).name where we can get the property variable name from a string. Didn't know about this feature but certainly saved many lines of code.

sbillinge · 2024-08-25T17:14:05Z

Lovely @bobleesj, Thanks so much!

bobleesj added 4 commits August 19, 2024 16:58

Make a dict of colors and use it for bg colors

1c6dd87

Use Enums to organize colors

161098e

Add tests for Colors and get_bg_colors

33caaac

Use classmethod to get bg_colors

5f424ee

bobleesj commented Aug 19, 2024

View reviewed changes

src/bg_mpl_stylesheets/tests/test_colors.py Show resolved Hide resolved

src/bg_mpl_stylesheets/colors.py Show resolved Hide resolved

sbillinge reviewed Aug 20, 2024

View reviewed changes

bobleesj added 2 commits August 20, 2024 08:22

Update news to add color enum

d481e8c

Make bg_colors in cycler immutable

55c6322

sbillinge reviewed Aug 20, 2024

View reviewed changes

bobleesj added 8 commits August 20, 2024 08:49

Return color name based on hex

032b175

Add test for retriving color from hex

9b69ccc

Capitalize hex for consistency

859a763

Change hex to color name in .png

1f2067f

Capitlize alphabets in hex colors

8804527

Update news to get color name from hex

68f8d1e

Capitalize to B0B0B0FF in test

a8e39ec

Add instruction for color name, color hex in readme

85d988d

bobleesj commented Aug 20, 2024

View reviewed changes

example/color_cycle.png Outdated Show resolved Hide resolved

bobleesj commented Aug 20, 2024

View reviewed changes

src/bg_mpl_stylesheets/colors.py Outdated Show resolved Hide resolved

sbillinge reviewed Aug 20, 2024

View reviewed changes

bobleesj added 6 commits August 20, 2024 16:33

Remove color change updates in news

853ddc5

Update readme with lowercase color name

bf7da33

Update color names like BG_BLUE to bg_blue

e911903

Update the color_cycle.png with lowercase names

31fe6fa

Change from b0b0b0 to B0B0B0

cdf22dd

Change from 1C6b0A to 1C6B0A

67db83e

bobleesj commented Aug 20, 2024

View reviewed changes

README.rst Outdated Show resolved Hide resolved

src/bg_mpl_stylesheets/colors.py Show resolved Hide resolved

bobleesj commented Aug 20, 2024

View reviewed changes

sbillinge reviewed Aug 20, 2024

View reviewed changes

Move news item on hex code capitalization under fixed

0ff689b

sbillinge reviewed Aug 20, 2024

View reviewed changes

news/colorupdate.rst Outdated Show resolved Hide resolved

bobleesj added 2 commits August 20, 2024 21:21

Bring * <news items> back under changed

ef7b548

Capitlize hex code in readme

bcb6003

bobleesj commented Aug 21, 2024

View reviewed changes

news/colorupdate.rst Show resolved Hide resolved

Fix A8b6C1 to A8B6C1 that caused test failure

7c0dd4d

sbillinge reviewed Aug 21, 2024

View reviewed changes

bobleesj and others added 4 commits August 21, 2024 17:32

Add example code in README for plot with a color

7a21f39

Clarify error message for get color name func

4c279f2

[pre-commit.ci] auto fixes from pre-commit hooks

c30f838

Shorten comment in README by removing 'the'

c48001c

sbillinge reviewed Aug 22, 2024

View reviewed changes

bobleesj and others added 5 commits August 24, 2024 16:48

Use enum's .value, .name to plot in readme

868ba1a

Merge branch 'color-select' of https://github.com/bobleesj/bg-mpl-sty…

e1aa641

…lesheets into color-select

Remove get color name class method

96e9023

[pre-commit.ci] auto fixes from pre-commit hooks

a705905

Remove toy example for plotting

ba05e3a

bobleesj commented Aug 24, 2024

View reviewed changes

simon final tweaks to documentation.

c899172

sbillinge merged commit abdf6fa into Billingegroup:main Aug 25, 2024
3 checks passed

bobleesj deleted the color-select branch August 25, 2024 17:55

Get color based on name using Enum #52

Get color based on name using Enum #52

Conversation

bobleesj commented Aug 19, 2024 • edited Loading

Motivation

Proposed solution

sbillinge left a comment

Choose a reason for hiding this comment

sbillinge commented Aug 20, 2024

sbillinge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobleesj left a comment • edited Loading

Choose a reason for hiding this comment

sbillinge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobleesj left a comment • edited Loading

Choose a reason for hiding this comment

bobleesj Aug 20, 2024 • edited Loading

Choose a reason for hiding this comment

sbillinge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbillinge left a comment

Choose a reason for hiding this comment

bobleesj left a comment

Choose a reason for hiding this comment

sbillinge commented Aug 21, 2024

bobleesj commented Aug 21, 2024

sbillinge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobleesj Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbillinge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobleesj commented Aug 21, 2024

sbillinge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobleesj commented Aug 22, 2024

bobleesj left a comment

Choose a reason for hiding this comment

bobleesj Aug 24, 2024 • edited Loading

Choose a reason for hiding this comment

sbillinge commented Aug 25, 2024

bobleesj commented Aug 19, 2024 •

edited

Loading

bobleesj left a comment •

edited

Loading

bobleesj left a comment •

edited

Loading

bobleesj Aug 20, 2024 •

edited

Loading

bobleesj Aug 21, 2024 •

edited

Loading

bobleesj Aug 24, 2024 •

edited

Loading