-
-
Notifications
You must be signed in to change notification settings - Fork 652
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
merge pants.util.objects.datatype with python 3 dataclasses #7074
Comments
#7226 adds a TODO in |
#7304 introduced |
3 tasks
#8181 should introduce |
cosmicexplorer
added a commit
that referenced
this issue
Oct 20, 2019
### Problem A first step to fixing #7074. We would like to be able to make use of python 3 `@dataclass`es, partially because they are the canonical way to make typed tuples in python now, and partially because it allows us to use mypy to resolve type errors at "compile" time instead of performing type checks at runtime when the tuple is constructed, which is what we do now, e.g.: https://github.com/pantsbuild/pants/blob/b4e316f4badc2703cea566267886d8b094d0f569/src/python/pants/util/objects.py#L120-L125 ### Solution - Edit the engine `README.md` to demonstrate how to use `@dataclass` and remove mentions of the old `datatype()`. - Add an integration test to verify that mypy will correctly detect type errors when constructing `@dataclass` objects. ### Result Users are able to use a very canonical `@dataclass` declaration syntax for engine params which is statically type-checked with mypy!
Eric-Arellano
added a commit
that referenced
this issue
Nov 5, 2019
Resolves #7074. Now that we always use `@dataclass` instead of `datatype`, and replaced `Collection.of(T)` with `Collection[T]`, it is safe to remove `datatype()`. We also remove `TypedCollection` as it is no longer in use. Instead, we use explicit type hints like `Tuple[str, ...]` or `Collection[T]`. We avoid a deprecation cycle because V2 engine code has always been experimental. ### Keeps `util/objects.py` We still keep `TypeConstraintError`, `Exactly`, `SuperclassesOf`, and `SubclassesOf`. These are used widely in the codebase and have no standard library equivalent. They also do not seem to cause problems for MyPy.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We have a
pants.util.objects.datatype
type constructor, which as of #5723 can add declarative type checking to its fields upon construction. In #6374, there was a consensus that we should move to using python 3 dataclasses, but that was only available for python 3.6+ (3.6 with a polyfill), which we didn't yet support in CI. After #7078 is resolved (which blocks this), we should be able to support their use in CI.Before that time, we should consider what
datatype()
provides us and what features we want or don't want from it vs python 3 dataclasses so that when the python 3 migration completes we can converge around the officially supported way of making typed named tuples.Note that the reason we use
datatype
is for low-overhead hashable objects to interface with the v2 engine -- see the engine README. It's not at all ridiculous to continue to have adatatype
constructor so we can maintain the qualities we need for v2 engine interaction, but it would be fantastic if we could switch the "backend" for our datatypes fromnamedtuple
todataclass
when we complete the migration to python 3.The text was updated successfully, but these errors were encountered: