Trying to do a whole directory in a diff , I can break this up if desired.
Details
Details
Diff Detail
Diff Detail
- Repository
- R1 dagster
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
| python_modules/dagster/dagster/check/__init__.py | ||
|---|---|---|
| 102 | Implicit return in function which does not return [misc] seems mypy doesnt know raise_with_traceback is NoReturn | |
| python_modules/dagster/dagster/core/execution/context/system.py | ||
| 370–379 ↗ | (On Diff #29977) | pretty sure this is right - fixed typing issues exposed while working |
Comment Actions
It would be nice to have a better story for these named tuples. Unfortunately, it might require mucking around with metaclasses to support overriding init if we want to do runtime type checking.
| python_modules/dagster/dagster/check/__init__.py | ||
|---|---|---|
| 102 | do we need raise_with_traceback now that we're on py3? | |
| python_modules/dagster/dagster/core/execution/plan/compute.py | ||
| 23 | I wonder if its worth defining a superclass here that users can use to annotate the return type of their solid functions? | |
| python_modules/dagster/dagster/core/execution/plan/inputs.py | ||
| 42–47 | Worth considering: class StepInput(typing.NamedTuple):
name: str
dagster_type: DagsterType
source: StepInputSource | |
| python_modules/dagster/dagster/core/execution/plan/plan.py | ||
|---|---|---|
| 717 | Should this be NoReturn? | |
Comment Actions
It would be nice to have a better story for these named tuples. Unfortunately, it might require mucking around with metaclasses to support overriding init if we want to do runtime type checking.
What i am thinking is that once we trust our internal code more - we can just change serdes to go through a static constructor method with check and drop the overload of __new__. The NamedTuple syntax here is more cumbersome but mypy does parse and understand the types when used this way.
| python_modules/dagster/dagster/core/execution/plan/inputs.py | ||
|---|---|---|
| 42–47 | for sure will happen in future diff | |
| python_modules/dagster/dagster/core/execution/plan/plan.py | ||
| 717 | it doesn't always throw - so the implicit None gets returned | |