jspahrsummers / adt

Algebraic data types for Python (experimental, not actively maintained)
MIT License
172 stars 14 forks source link

Mixed-case case names not accepted by .match(), but README says they're fine #41

Open kodo-pp opened 3 years ago

kodo-pp commented 3 years ago

The README says:

It's conventional to declare your fields with ALL_UPPERCASE names, but the only true restriction is that they cannot be lowercase.

If I understand this correctly, it means that the only kind of names that are invalid are names that are fully in lowercase. That is, FOO_BAR, FooBar, and foo_Bar are valid names, but foobar and foo_bar aren't.

However, match method does not seem to work nicely with names that are not fully in uppercase. The following code is supposed to work:

from adt import adt, Case

@adt
class Foo:
    Bar: Case
    Baz: Case

value = Foo.Bar()
print(value.match(
    bar=lambda: 'bar',
    baz=lambda: 'baz',
))

But it fails with the error: ValueError: Unrecognized case BAR in pattern match against <<class '__main__.Foo'>.Bar: None> (expected one of dict_keys(['Bar', 'Baz'])).

The code of match seems to convert the names of variants to upper case, which may not be correct when original names contain lowercase letters (provided that such names are allowed).

I see two possible solutions here.

  1. Forbidding variant names not in the format ALL_UPPERCASE, stating this in the README and perhaps enforcing this rule in the decorator in order to prevent accidental mistakes. This may be backwards-incompatible, but is probably the easiest solution. As a compromise, it is possible to deprecate names containing lowercase characters and possibly to warn about them in the decorator.
  2. Modifying the code to accept non-fully uppercase names. This is tricky because we either have to continue accepting case-insensitive variant names in match and somehow convert them to their original case, or accept only names in their original case (that is, one would only be able to write something like value.match(Bar=..., Baz=...), not value.match(bar=..., baz=...)). The former may be tricky to implement, and the latter would probably totally break backward compatibility.

Out of these two, I personally prefer the first solution, but it's just my opinion on this problem.

P.S. I used the term variant as a synonym of case here to avoid confusion between lower/upper case and ADT cases.