liesel icon indicating copy to clipboard operation
liesel copied to clipboard

Rename Data -> Const; keep old name as an alias

Open jobrachem opened this issue 1 year ago • 3 comments

This PR goes back to a suggestion made by @Seb-Lorek :

I find the naming of the node "Data" a bit unfortunate. I would have thought of Fixed or something like that. When you think of Data, you generally think of oberved data in the model. Also the description: "A Node subclass that holds constant data.". Doesn't help, it reminded me more of observed data. (Thought the description somehow fit: "Constant values better.")

I am not sure whether I am entirely happy with Const, because this class is also used as the lsl.Var.value_node of strong variables. If they are parameters, the values are not actually constant, but updated during sampling. So maybe Value would be better than Const. What do you guys think @GianmarcoCallegher @wiep? Of course this PR is not super urgent, so no need to prioritise it right now,

jobrachem avatar Dec 07 '23 10:12 jobrachem

The name Const would suggest that the value is constant and immutable. From the two options, I like Value better.

wiep avatar Jan 10 '24 21:01 wiep

Is this ready for a review?

GianmarcoCallegher avatar Feb 08 '24 21:02 GianmarcoCallegher

No, the name change is not implemented in the way it is supposed to be.

jobrachem avatar Feb 08 '24 21:02 jobrachem

@GianmarcoCallegher this is finally ready for review 😊

jobrachem avatar Aug 14 '24 20:08 jobrachem

Since the docs actions had not completed for a while, I pushed a minor commit (an addition to the gitignore) to trigger a new build.

jobrachem avatar Aug 28 '24 16:08 jobrachem