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

LabeledEnum needs support for labels in grouped values #151

Closed
jace opened this issue Oct 30, 2017 · 2 comments
Closed

LabeledEnum needs support for labels in grouped values #151

jace opened this issue Oct 30, 2017 · 2 comments

Comments

@jace
Copy link
Member

jace commented Oct 30, 2017

A need for labels in grouped values became apparent in #150. Syntax like this would be nice:

class POSTSTATUS(LabeledEnum):
    DRAFT =        (0,  __("Draft"))         # Being written
    PENDING =      (1,  __("Pending"))       # Pending email verification
    CONFIRMED =    (2,  __("Confirmed"))     # Post is now live on site
    REVIEWED =     (3,  __("Reviewed"))      # Reviewed and cleared for push channels

    UNPUBLISHED = {DRAFT, PENDING}, __("Unpublished")
    LISTED = ({CONFIRMED, REVIEWED}, __("Listed"))

From an implementation standpoint, the two grouped entries (identical in effect thanks to Python's automatic list packing) will show up as a tuple, but with the first element being a set instead of a non-set (integer, string, etc). In this case the metaclass needs to parse the set for inner values. Relevant code:

if key != '__order__' and isinstance(value, tuple):
# value = tuple of actual value (0), label/name (1), optional title (2)
if len(value) == 2:
labels[value[0]] = value[1]
attrs[key] = value[0]
elif len(value) == 3:
labels[value[0]] = NameTitle(value[1], value[2])
attrs[key] = value[0]
elif key != '__order__' and isinstance(value, set):
# value = set of other unprocessed values
attrs[key] = {v[0] if isinstance(v, tuple) else v for v in value}

@jace
Copy link
Member Author

jace commented Oct 30, 2017

There are complications:

  1. The labels dictionary can't have a set as the key. The set must be converted into a frozenset. However, frozensets aren't instances of a set (as per isinstance) and so there could be downstream breakage. If we use a frozenset (or a tuple) only for the key, without changing the actual value, it breaks dictionary access to the label (for example, POSTSTATUS[POSTSTATUS.UNPUBLISHED]).
  2. Dictionary enumeration is meant to provide a list of keys that are valid values for setting a status flag. If we provide grouped values in this enumeration, it breaks another downstream use case.

Perhaps grouped value labels should be stored separately, and lookup should happen as a fallback when __getitem__ notices the key is a set or frozenset.

Given the increased complexity to LabeledEnum's implementation, this ticket should be parked until #150 establishes a clear need for group labels.

@jace
Copy link
Member Author

jace commented Nov 14, 2017

Closing this as #150 did not find a use case for labels in groups. Our use case for labels remains the UI display of the current primary state. Grouped states are tested against, but not displayed (so far). Will re-open if this changes.

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

No branches or pull requests

1 participant