tests: serialize pipeline file #3985
Conversation
serialize: make params ordered
skshetry
added
refactoring
| @@ -70,16 +82,15 @@ def _serialize_params(params: List[ParamsDependency]): | |||
| dump = param_dep.dumpd() | |||
| path, params = dump[PARAM_PATH], dump[PARAM_PARAMS] | |||
| if isinstance(params, dict): | |||
| k = list(params.keys()) | |||
| k = sorted(params.keys()) | |||
There was a problem hiding this comment.
Before, we were not sorting params keys for pipeline file before, only for the lock.
| [("plots", {"plot": True}), ("metrics", {"metric": True}), ("outs", {})], | ||
| ) | ||
| def test_outs_and_outs_flags_are_sorted(dvc, typ, extra): | ||
| stage = create_stage(PipelineStage, dvc, deps=["input"], **kwargs) |
There was a problem hiding this comment.
Maybe, we should implement {metrics,plots}[_persist][_no_cache] that are missing?
There was a problem hiding this comment.
@skshetry Could you elaborate?
There was a problem hiding this comment.
On .dvc and dvc.yaml files, we allow metrics and plots to be cache: False and persist: True. But, it cannot be set from outside (eg: fill_stage_outputs and dvc run). So, I was just saying that maybe we should implement that for run as well.
There was a problem hiding this comment.
@skshetry Let's not create those dvc run options for now, it is crowded there as it is. We clearly need to consider a different CLI approach, as metrics_persist_no_cache is just too ugly and doesn't scale if we introduce yet another option 🙂 We've been discussing something like using a comma to join those fstab-style, but, again, not worth bothering with that for now.
|
For the record: snap is failing for unrelated reasons. |
This PR adds sorting for params keys as well. Before, it was only getting sorted for lockfile.
Rest are tests and a bit of refactoring that does not change anything.
❗ I have followed the Contributing to DVC checklist.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here. If the CLI API is changed, I have updated tab completion scripts.
❌ I will check DeepSource, CodeClimate, and other sanity checks below. (We consider them recommendatory and don't expect everything to be addressed. Please fix things that actually improve code or fix bugs.)
Thank you for the contribution - we'll try to review it as soon as possible. 🙏
Part of #3693