why does pants define its own wrapper around the s...
# development
h
why does pants define its own wrapper around the stdlib
Enum
?
e
Apparently to support exhaustive match semantics. It looks like that code in our Enum subclass could just be extracted as a function to work with stdlib Enum; IOW: I'm not sure we need this and it probably is just an artifact of refactoring when our Enum class did more.
h
It was a compromise made when migrating away from
enum()
to avoid losing the functionality of
.resolve_for_enum_variant
. Originally we used a simple dictionary lookup, but several reviewers wanted the exhaustiveness check to be preserved Converting match() from a method to a function could make sense!
e
Yeah - looks like we could just kill our type and lift the match function:
Copy code
$ python
Python 3.7.4 (default, Oct  4 2019, 06:57:26) 
[GCC 9.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from enum import Enum, auto
>>> from typing import Dict, TypeVar
>>> 
>>> 
>>> class Choices(Enum):
...   A = auto()
...   B = auto()
... 
>>> 
>>> E = TypeVar('E', bound=Enum)
>>> R = TypeVar('R')
>>> 
>>> 
>>> class EnumMatchError(ValueError):
...   """Issue when using match() on an enum."""
... 
>>> 
>>> class UnrecognizedMatchError(EnumMatchError):
...   """A value is used that is not a part of the enum."""
... 
>>> 
>>> class InexhaustiveMatchError(EnumMatchError):
...   """Not all values of the enum specified in the pattern match."""
... 
>>> 
>>> def match(enum_value: Enum, arms: Dict[E, R]):
...   enum_type = type(enum_value)
...   all_values = [e for e in enum_type]  
...   unrecognized_values = [value for value in arms if value not in all_values]
...   if unrecognized_values:
...     raise UnrecognizedMatchError(
...       f"Match includes values not defined in the enum. Unrecognized: {unrecognized_values}"
...     )
...   missing_values = [value for value in all_values if value not in arms]  
...   if missing_values:
...     raise InexhaustiveMatchError(
...       f"All enum values must be covered by the match. Missing: {missing_values}"
...     )
...   return arms[enum_value]
... 
>>> 
>>> match(Choices.A, {Choices.A: 42})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 12, in match
__main__.InexhaustiveMatchError: All enum values must be covered by the match. Missing: [<Choices.B: 2>]
>>> match(Choices.A, {Choices.A: 42.0, Choices.B: 1/137})
42.0
>>>
🎉 1
a
wow, i hadn’t considered that at all
a top-level function even sounds better as an idea like the enum isn’t doing anything itself it’s just a flag
💯 1
i can also put up — and follow through and merge — a PR doing this
i was actually about to sleep it’s like that “i sleep / i wake” meme
h
Also does this belong in collections.py or in objects.py? I realized that after we remove datatype, we can get MyPy working with objects.py. Part of my original motivation for putting Enum in collections was because I thought there were more blocking issues in objects.py until realizing this that we only need to remove datatype()
Same with
@frozen_after_init
. Maybe belongs in
objects.py
over
meta.py
?
a
meta.py
? i feel more comfortable putting stuff there if it's specifically to operate on stdlib types
hm
no i think yours is better
meta.py
maybe could be like "literal extensions to stdlib stuff like `classproperty`" and
objects.py
more like "the pants codebase uses these methods/decorators"
with
engine/objects.py
being that, but for the engine specifically somehow
h
Use
meta.py
for both? I’m not convinced either way and totally think it’s worth revisiting. I know it results in a bit more temporary churn, but hopefully these APIs will stabilize after one or two more weeks of tweaks like John&/ proposal
a
we might be able to kill
objects.py
after we kill
datatype
👍 1
i don't think `TypeConstraint`s are used anywhere else (or they can be replaced without too much issue), would have to check though
i'm thinking about it and i like the idea of: (1) move decorators and
match
into
meta.py
(2) kill
datatype
(3) remove
TypeConstraint
usages (4) kill
objects.py
not too opinionated about it
👖 1
❤️ 1
💯 1
h
with
engine/objects.py
being that, but for the engine specifically somehow
About that file, going to take another look at Collection.of() this weekend after having to revert Collection[X]. I realized in the revert that there’s very little difference between Collection[X] and Tuple[X, ...]. We might be able to simplify by using the latter. Still testing if it will work
a
i think we should absolutely use
Tuple[X, ...]
mostly because i think i was conflating like "the specific
Collection.of()
factory" with "the ability to wrap types in other types" in my head the last time we discussed this
🔥 1
h
(Kind of related, my last day teaching is now Nov 1 so I am going to have much more time after two weeks to get our type coverage much higher once we finish these objects.py changes! Hoping to get us to majority using
partially_type_checked
)
✍️ 1
🔥 1
Me too with
Collection.of()
! I was really confused by the difference
a
your wish is my mind's demand: https://github.com/pantsbuild/pants/pull/8504 (move enum pattern matching into its own top-level function in
meta.py
)
🎉 1