search

scipp / sciline

Build scientific pipelines for your data
https://scipp.github.io/sciline/
BSD 3-Clause "New" or "Revised" License
10 stars 2 forks source link

Extend dict-like interface for parameters #185

Open jl-wynen opened 1 month ago

jl-wynen commented 1 month ago

Pipeline currently only supports __setitem__. For parameters, it seems natural to have

However, they clash with __getitem__. And it is unclear to me how most should be implemented with regard to providers.

However, at a minimum, I think we should add update.

And we should make sure that Python's default iterator behaviour raises meaningful errors. Currently, it fails because __getitem__ cannot be called with integers. E.g., list(pipeline) and x in pipeline raise KeyError: "Node '0' does not exist in the graph.".

SimonHeybrock commented 1 month ago

I think adding these methods makes perfect sense, but they should not be specific to parameters. Instead, they should behave consistently with the graph operations provided by __getitem__, i.e., no clash?

For example, __contains__ will return True if the node is in the graph, regardless of whether a value is set.

jl-wynen commented 1 month ago

And what about deletion? Would that remove the entire subgraph that leads to a given key? because that is what __getitem__ returns.

SimonHeybrock commented 1 month ago

Exactly, it would remove the subgraph (all nodes connected to only this one). I think this is probably already implemented as part of __setitem__, since that is what it has to do internally?