Make a typed datatype #5716
Comments
|
Oh, that's a nice idea |
|
I happened to stumble upon @jsirois's comment in #5580 and ensuing discussion today and it seems like relevant context, in the sense of: if we're doing type checking with some well-defined magic in If we're doing type checking, would we assume edit: see the changes to |
|
I'd be interested in seeing benchmarks on this one, as we create a lot of objects.
We already do, in the sense that when the engine invokes rules, it guarantees not to invoke them with arguments of the wrong type. Primarily: select "noops" (...but should probably fail) if an output has the wrong type. |
|
I don’t know which code paths the benchmark should measure, but it seems
important to establish what with all this composable metaprogramming. I
would almost definitely want my rule to fail if the output is the wrong
type, I think (thanks for the tip on where to look).
…On Wed, Apr 18, 2018 at 03:41 Stu Hood ***@***.***> wrote:
I'd be interested in seeing benchmarks on this one, as we create a *lot*
of objects. datatype is also used outside of the engine in a few places.
------------------------------
if we're doing type checking with some well-defined magic in datatype
ctors, would we want to apply that process to e.g. @rule
<https://github.com/rule> arguments?
We already do, in the sense that when the engine invokes rules, it
guarantees not to invoke them with arguments of the wrong types, for a few
reasons: 1) select "noops" (...but should probably fail) if an output
argument has the wrong type.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#5716 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABPqT99XwU-YziOKSWfUR5O_TQ_qo-Ewks5tpxhmgaJpZM4TYqgx>
.
|
|
To be clear, @rules failing (rather than nooping) for the wrong return type
is something that the engine code should be doing now: it's not something
that the datatype constructor can do anything about though: would need to
be checked on the rust side. And I think it has one fidgety bit to figure
out related to unions.
On Wed, Apr 18, 2018, 12:51 PM Danny McClanahan <notifications@github.com>
wrote:
… I don’t know which code paths the benchmark should measure, but it seems
important to establish what with all this composable metaprogramming. I
would almost definitely want my rule to fail if the output is the wrong
type, I think (thanks for the tip on where to look).
On Wed, Apr 18, 2018 at 03:41 Stu Hood ***@***.***> wrote:
> I'd be interested in seeing benchmarks on this one, as we create a *lot*
> of objects. datatype is also used outside of the engine in a few places.
> ------------------------------
>
> if we're doing type checking with some well-defined magic in datatype
> ctors, would we want to apply that process to e.g. @rule
> <https://github.com/rule> arguments?
>
> We already do, in the sense that when the engine invokes rules, it
> guarantees not to invoke them with arguments of the wrong types, for a
few
> reasons: 1) select "noops" (...but should probably fail) if an output
> argument has the wrong type.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#5716 (comment)
>,
> or mute the thread
> <
https://github.com/notifications/unsubscribe-auth/ABPqT99XwU-YziOKSWfUR5O_TQ_qo-Ewks5tpxhmgaJpZM4TYqgx
>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#5716 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAC2lBBYmTYzLZ9sGtJ0t7tEjEJ2z2QRks5tpxqqgaJpZM4TYqgx>
.
|
|
That was what I was thinking as well, that the result of a rule would be typechecked by the engine instead of through any |
|
It's also possible (if we're talking about a way to easily define simple datatypes) it might be appropriate to apply this to options (e.g. a class could be declared which ensures its argument is an existing file/directory). While nothing would change from the pants user's point of view, it might be easier for implementors of subsystems and tasks to do appropriate error checking if we could match args to more complex types. |
Options already do this. See |
…me argument (#5723) ### Problem See #5716. We had to introduce a ton of boilerplate to check argument types in some very simple usage of `datatype` subclasses in tests in #5703. This is one way to make that easier. ### Solution - Move `TypeConstraint` and subclasses from `src/python/pants/engine/addressable.py` to `src/python/pants/util/objects.py`, where they probably should have been anyway, to avoid an import cycle. - Remove the class name argument from `datatype()` (and update all call sites). - Allow specifying a tuple `('field_name', FieldType)` in the list of fields to `datatype()` to have the field type-checked by an `Exactly(FieldType)` type constraint at construction. - Override `__str__()` and `__repr__()` for `datatype` objects, and add testing for the str and repr. - Add testing for informative error messages in `datatype` construction. ### Result We can now concisely declare `datatype` objects without having to repeat the class name, and can opt-in to a type check for specific fields.
|
Thanks! |
We're making a lot of the datatypes to exist as v2 arguments to rules, and a lot of them have type-checks on their arguments. We should make it easy to write something like:
to reduce boilerplate.
The text was updated successfully, but these errors were encountered: