Var constructors: new_param, new_obs, new_calc, new_value

[jobrachem]

For a while now, I have been a little unsatisfied with two things:

1. Incsonsistent UI: Var class vs. param and obs functions. See also Rename Param and Obs #130
2. The status of the Data (or, in Rename Data -> Value; keep old name as an alias #170 : Value) node vs. Var: Which should you use?

I think there is a satisfying solution:

1. Implement constructors for Var that fullfill the tasks above.
2. Guide users to towards using Var.new_<constructor> as a default.

If I recall correctly, using constructors has been proposed before by @wiep.

This PR implements four constructors.

Var.new_param

Supersedes lsl.param()

class Var:

    @classmethod
    def new_param(
        cls, value: Any, distribution: Dist | None = None, name: str = ""
    ) -> Self:
        ...

Var.new_obs

Supersedes lsl.obs()

class Var:
    @classmethod
    def new_obs(
        cls, value: Any, distribution: Dist | None = None, name: str = ""
    ) -> Self:
        ...

Var.new_calc

New. Having this constructor available makes the interface more consistent. Users can simply start typing Var.new_ and see the available options via auto-completion. It also reduces errors from confusion about nesting Calc objects in Var constructors.

class Var:
    @classmethod
    def new_calc(
        cls,
        function: Callable[..., Any],
        *inputs: Any,
        name: str = "",
        _needs_seed: bool = False,
        update_on_init: bool = True,
        **kwinputs: Any,
    ) -> Self:
        ...

Var.new_const

New. Like with Var.new_calc, having this constructor makes the interface more consistent and supports a good habit.

class Var:
    @classmethod
    def new_const(cls, value: Any, name: str = "") -> Self:
        ...

Todo

This is a draft PR for a first discussion. Even if it remains unchanged, documentation has to be added if it ends up being merged.

• Add documentation
• Add deprecation warnings to param and obs
• (Maybe) add a warning to using the default Var constructor

[jobrachem]

Discussion notes

• The three constructors .new_param, new_obs and new_calc appear to be unproblematic.
• There's doubt about the need for and naming of .new_const. The naming could imply that it is immutable, which it is not. It is possible to change the name.
• We could go forward with this constructor, too, but add a disclaimer about it being experimental.

What is the purpose of .new_const?

• It is equivalent to using the default lsl.Var without specifying a distribution.
• So it holds a value that is neither observed nor a parameter. In most cases it is not crucial to explicitly make such a value immutable.
• It should encourage the good habit of using the .new_ constructors.

Maybe .new_const just needs a better name. Maybe lsl.Var.new_value? That would provide a nice parallel to how lsl.Var.new_calc wraps a lsl.Calc.

[jobrachem]

This PR is now ready for review. In addition to th new constructors, I added and updated the documentation:

• Var docstring
• Docstrings of the four new constructors
• Calc docstring
• Representation on the "model building" overview page in the docs
• I also ended up tweaking the Model docstring a bit, and gave the Model class a more prominent position on the "model building" overview page.

The helper functions lsl.param and lsl.obs are marked as deprecated and emit a FutureWarning now. They are announced as scheduled for removal in v0.4.0. Currently we are moving towards v0.2.10.

During the review, please pay attention not only to the pure code implementation, but also to the clarity of the documentation 🙏

[jobrachem]

@GianmarcoCallegher gentle reminder :)

[jobrachem]

@GianmarcoCallegher As we just discussed, the test action is currently failing due to timeout. What is surprising is that I did not change any code between the previous test run and the current test run. I just re-ran the action. Locally, the tests run in <150s. This is weird to me.

[jobrachem]

See #213 regarding the tests timing out.

[jobrachem]

@GianmarcoCallegher the testing issue is solved for the moment, and the PR can be reviewed :)

[GianmarcoCallegher]

All clear. Just a little comment