- Added some mypy types, including making Partitions generic.
- schedule_partition_range now returns a list of partitions instead of a function that returns a list of partitions.
- PartitionParams -> Partitions. I don't 100% love Partitions as a replacement, but IMO it gets the meaning across better. PartitionParams makes me expect a dumb struct of parameters to some fancy function.
Details
Details
bk
Diff Detail
Diff Detail
- Repository
- R1 dagster
- Branch
- partition-grok (branched from master)
- Lint
Lint Passed - Unit
No Test Coverage
Event Timeline
Comment Actions
I prefer partition_params to just partitions, but not in a die-on-the-hill fashion
| python_modules/dagster/dagster/core/definitions/partition.py | ||
|---|---|---|
| 129 | PartitionsFactory? :) get_partitions() on Partitions feels weird | |
| 290–301 | we could probably do a better job explaining here the invariant below that one of 'partition_fn' or 'partitions' must be supplied | |
| python_modules/dagster/dagster/core/definitions/partition.py | ||
|---|---|---|
| 137 | would probably be nice to have a check here | |
Comment Actions
@rexledesma @dgibson I added one more change, which is to rename TimeBasedPartitions to ScheduleTimeBasedPartitions - this will help disambiguate with some of the things I'm adding for the crag partitions revamp. Also added the check you requested @rexledesma. Would PartitionsDefinition be better than Partitions?
Comment Actions
this looks good to me, one question inline
| python_modules/dagster/dagster/core/definitions/partition.py | ||
|---|---|---|
| 433 | is this line right? I thought it was still either returning a single thing or a list of things, but the thing in both was the same | |
| python_modules/dagster/dagster/core/definitions/partition.py | ||
|---|---|---|
| 433 | codemod error. will fix | |