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

"User friendly" enums #1181

Open
Huite opened this issue Aug 30, 2024 · 5 comments
Open

"User friendly" enums #1181

Huite opened this issue Aug 30, 2024 · 5 comments
Assignees
Milestone

Comments

@Huite
Copy link
Contributor

Huite commented Aug 30, 2024

Basically a follow up of #416:

We've briefly discussed this before: Enums are a great way to enumerate options, but a lot of our users aren't familiar with them. The issue with enums is also that you need to import the relevant enums from the right namespace. A pragmatic solution is to dynamically force inputs to enums, thereby checking them as well.

This gives decent errors:

from enum import Enum


class Color(Enum):
    RED = 0
    GREEN = 1
    BLUE = 2

color = Color("YELLOW")

ValueError: 'YELLOW' is not a valid Color

Ideally, it wouldn't tell you that it's wrong, but what the right entries are. This is especially helpful with typos.

from enum import Enum
from typing import Union, Type, TypeVar


E = TypeVar('E', bound='FlexibleEnum')


def _show_options(options: Enum) -> str:
    return "\n * ".join(map(str, options.__members__))


class FlexibleEnum(Enum):
    @classmethod
    def from_value(cls: Type[E], value: Union[E, str]) -> E:
        if isinstance(value, cls):
            return value
        elif isinstance(value, str):
            try:
                return cls.__members__[value]
            except KeyError:
                pass

        raise ValueError(
            # Use __repr__() so strings are shown with quotes.
            f"{value.__repr__()} is not a valid {cls.__name__}. "
            f"Valid options are:\n * {_show_options(cls)}"
        )
        

class Color(FlexibleEnum):
    RED = 1
    GREEN = 2
    BLUE = 3
    

Color.from_value("YELLOW")
    

Color.from_value("YELLOW")
ValueError: 'YELLOW' is not a valid Color. Valid options are:
 * RED
 * GREEN
 * BLUE

Another advantage is that regular enums accept integer values.

E.g. one of the current enums:

class ALLOCATION_OPTION(Enum):
    stage_to_riv_bot = 0
    first_active_to_elevation = -1
    stage_to_riv_bot_drn_above = 1
    at_elevation = 2
    at_first_active = 9  # Not an iMOD 5.6 option

In general, I don't think we want user facing functions to support something like .allocate(option=0). A default Enum will support ALLOCATION_OPTION(0).

But with the FlexibleEnum.from_value(), it won't (which is good):

Color.from_value(1)

ValueError: 1 is not a valid Color. Valid options are:
 * RED
 * GREEN
 * BLUE

So my suggestion is to replace the Enums with these FlexibleEnums (or a better name), and preferable the same for all strings literals that we support. Then inside of the function:

def f(arg, option: str | ThisOptionEnum):
     option = ThisOptionEnum.from_value(option)
     ...

This ensures the option is validated and that a clear error message listing the available valid options is printed.

@Manangka
Copy link
Collaborator

Manangka commented Aug 30, 2024

I like the idea. Some of my thoughts

In the first code snippet you define the Color class as class Color(Enum). You assign a color as:
color = Color("YELLOW")
which throws an exception. However a valid value also throws an exception:
color = Color("RED")

What i gather from the python documentation calling the class like that is called the function-call syntax for creating an enum. Its an alternative way to the class defintition you use. It isn't used to create a specific enum member
https://docs.python.org/3/library/enum.html


Preferably the new Enum should be a drop in replacement of the original enum.
It would be nice that when someone uses it like Color.Yellow it would give your error message.


I do like your from_value method as well. Its similar to C# Enum.Parse method to create an Enum from a string.
I think i would it a bit differently in the code:
option = option if isinstance(option, Color) else Color.Parse(option)

@Huite
Copy link
Contributor Author

Huite commented Aug 30, 2024

You're right: I got my first example mixed up, what I should/could have written is:

class Color(Enum):
    RED = "RED"
    GREEN = "GREEN"
    BLUE = "BLUE"

This does accept Color("RED"), but the duplication feels rather silly here.

This works:

class Color(Enum):
    RED = 0
    GREEN = 1
    BLUE = 2

Color(0)  # -> Color.RED

But is kind of anti-feature for our usage.

It would be nice that when someone uses it like Color.Yellow it would give your error message.

This is doable, although not that easy. It needs a metaclass.

Adding a parse method is a good idea.

Here's basic implementation:

from enum import Enum, EnumMeta
from typing import Any, Type, TypeVar, Union

E = TypeVar("E", bound="FlexibleEnum")


def _show_options(options: Type[E]) -> str:
    return "\n * ".join(map(str, options.__members__))


class AttributeErrorMeta(EnumMeta):
    def __getattr__(cls, name: str) -> Any:
        try:
            return cls.__members__[name]
        except KeyError:
            raise AttributeError(
                f"{name} is not a valid {cls.__name__}. "
                f"Valid options are:\n * {_show_options(cls)}"
            )


class FlexibleEnum(Enum, metaclass=AttributeErrorMeta):
    @classmethod
    def parse(cls: Type[E], value: str) -> E:
        try:
            return cls.__members__[value]
        except KeyError:
            raise ValueError(
                # Use __repr__() so strings are shown with quotes.
                f"{value.__repr__()} is not a valid {cls.__name__}. "
                f"Valid options are:\n * {_show_options(cls)}"
            )

@Manangka
Copy link
Collaborator

Manangka commented Sep 2, 2024

Nice!

I just tested this:

# Existing value
Color.RED
<Color.RED: 0>

# Non-existing value
Color.YELLOW
Traceback (most recent call last):
KeyError: 'YELLOW'

During handling of the above exception, another exception occurred:

AttributeError: YELLOW is not a valid Color. Valid options are:
 * RED
 * GREEN
 * BLUE

Which gives a clear error message.

I think it is fine that a metaclass is needed to get this output.
Its a small addition to a class that wont change much.
It is also something that can be well tested using unit tests

@Huite
Copy link
Contributor Author

Huite commented Sep 13, 2024

FYI, I've included an implementation in pandamesh here:

https://github.com/Deltares/pandamesh/blob/main/pandamesh/enum_base.py

I've maintained the from_value method there because it saves on a lot of duplication in the places where it's used.

@Huite
Copy link
Contributor Author

Huite commented Sep 13, 2024

I was also struggling with rendering the enums docs in an appropriate manner, I needed a custom template in the end:

https://github.com/Deltares/pandamesh/blob/main/docs/_templates/enums.rst

This is included in the index:

.. autosummary::
   :toctree: api/
   :template: enums.rst

    DelaunayAlgorithm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🤝 Accepted
Development

No branches or pull requests

3 participants