SofaPython3 icon indicating copy to clipboard operation
SofaPython3 copied to clipboard

[Binding/Sofa.Config] Add Sofa.future & Sofa.Config

Open damienmarchal opened this issue 4 years ago • 8 comments

Implement a control mecanism to activate/de-activate feature of the SofaPython3 binding so we have more freedom to experiment.

Example of use (taken from PR #173):


from Sofa.Lifecycle import __feature__
def createScene(root):
     with __new_feature__("object_auto_init",True):
           root.addObject("MechanicalObject", position=[12,3])  # init() is now called automatically

The mechanisme is composed of three parts:

  • SofaPython3::Lifecycle (the c++ part)
  • SofaPython3::Binding::Lifecycle (the python binding of the c++ Config)

To add a new feature:

In binding/Sofa/package/lifecycle.py, add you feature at the begnining as indicated in the file.

Then in your c++ code you can then test if the feature is activated by doing

if( sofapython3::lifecycle::features::get("global_auto_init") )
	....

Or in your python code you can also test if the feature is activated by doing

import Sofa.Lifecycle
if Sofa.Lifecycle.has_feature("global_auto_init"):
	print("The feature is activated")

damienmarchal avatar Sep 15 '21 22:09 damienmarchal

@hugtalbot here is the feature. I think it should be used at least in PR #173 and few other.

damienmarchal avatar Sep 15 '21 22:09 damienmarchal

@damienmarchal you are changing the name in the latest commit right? Why lifetime ? I just see a lifecycle in SOFA but related to the componentState, therefore unrelated to this Future feature.

Could you explain ? I thought Future was a standard keyword in the python world

hugtalbot avatar Sep 27 '21 13:09 hugtalbot

Oops, I missread "lifecycle" in the sofa code base (which is not related to componentState).

Future is the common name in python...but as we already have a namespace for feature management and software lifecylce management in Sofa I think it is better to stick to the sofa namespace.

damienmarchal avatar Oct 06 '21 10:10 damienmarchal

@hugtalbot I now rename everything so it match the sofa naming convention: lifecycle.

I also renamed the contextmanager __feature__ into __new_feature__... in case one day we decide to have something like __old_feature__ if one day we decide to provide on demand backward compatibility.

EDIT: The CI is failing because we need to merge PR #https://github.com/sofa-framework/SofaPython3/pull/206

damienmarchal avatar Oct 14 '21 08:10 damienmarchal

Hi @damienmarchal I have a few questions regarding this PR.

  1. Adding features in lifecycle::features
    • Why? What is considered a "new feature" and thus should be registered in lifecycle::features? What are the criteria?
  2. Removing features from lifecycle::features
    • How? Are all lifecycle::features meant to be used as normal features at some point?
    • Why? What are the agreed reasons for removing features? The expiration date (if any)? Something else?
    • When? How long does a feature stay registered in lifecycle::features? Is there an expiration date?
    • Who? Who is supposed to do that? Who is in charge of maintaining the lifecycle::features list?
  3. Other questions
    • Could we have a message telling the user he is using a "new_feature"? He might not know if he runs the scene of someone else.

Thanks.

guparan avatar Oct 25 '21 08:10 guparan

Nice day, I Have questions about a pending PR Thanks @guparan

Adding features in lifecycle::features

Q: Why? What is considered a "new feature" and thus should be registered in lifecycle::features? What are the criteria? A: at first sight I would say that two good candidates for explicit activations of new feature using context manager are:

  • features that breaks compatibility with existing code base (eg: PR #45 or #173)
  • features that are not breaking but that exposes an API that we know in advance will change but don't know when and how (eg: PR #148 that is waiting since May). So this is kind of "fragile" feature :) This would allow their integration and makes them usable while warning users that things may change.

Apart from that I would not recommand to use this kind of mecanism for integration of new feature that are neither breaking nor fragile.

Removing features from lifecycle::features

Q: How? Are all lifecycle::features meant to be used as normal features at some point? A: By adding a context manager working the reverse way as new_feature. Example of evolution of the "init by default" behavior in Sofa. As it is breaking we first deploy it through explicit activation by users.

## I really want the new behavior. 
with Sofa.lifecycle.__new_feature__("auto_init"):
       createPartOfSceneNewStyle(root)
createPartOfSceneWithOldStyle(root)

When the feature is integrated, there is no need for the with statement. If the with new_feature is used we can now emit a warning explainging this is not needed anymore as the feature is now a properly supported one.

createPartOfSceneNewStyle(root)

Finally, if the scene want to continue using some parts with the old behavior we can explicitly fall back to the old behavior.

createPartOfSceneNewStyle(root)

## I'm to lazy to update my scene but I need to make it work now !
with Sofa.lifecycle.__deprecated_feature__("no_auto_init"):   
      createPartOfSceneWithOldStyle(root)

Thanks to this kind of lifecyle mecanism we then have a the infrastructure to handle a smooth transition and api evolution (which compared to the no-effort we have done for python2 to pyton3 is a big improvement).

Q: Why? What are the agreed reasons for removing features? The expiration date (if any)? Something else? A: The decision to remove a feature is strictly the same as for the other parts of Sofa. In general this is when the sofa-dev team agreed it is time for that and when there is alternatve and a good migration plan.

Q: When? How long does a feature stay registered in lifecycle::features? Is there an expiration date? A: Same as for other sofa lifecyle/deprecation/removal dates, this is decided by the dev-team.

Q: Who? Who is supposed to do that? Who is in charge of maintaining the lifecycle::features list? A: Same as for other sofa lifecycle management. In general this is the one that make a PR introducing a breaking feature, or removes a deprecated one that is in charge of adding the transitional strategy as part of the PR.

Q: Could we have a message telling the user he is using a "new_feature"? He might not know if he runs the scene of someone else. A: Every message combination is possible. I will make a proposal in which we emit msg_info().

damienmarchal avatar Oct 25 '21 16:10 damienmarchal

Hi @damienmarchal

features that breaks compatibility with existing code base

This sounds to me like the master branch isn't it ? I would not really agree on this criteria.

features that are not breaking but that exposes an API that we know in advance will change but don't know when and how

Agreed here, but it sounds rather like something experimental, since unsure to be finally merged in master. Don't you think? I also do not think #148 falls into this case.

Regarding #173, the question is rather to me do we really want this feature in SP3? I can not see why an automatic init should be hidden. Anyone else @sofa-framework/reviewers-sofapython3 has an opinion on this one?

hugtalbot avatar Oct 26 '21 13:10 hugtalbot

(From SOFA dev) During the SOFA dev meeting, the team agreed on setting up:

  • a mechanism to smoothly integrate new breaking features in SofaPython3 by putting it behind a ON/OFF switch (ON by default). It should always be coupled with a deprecated mechanism if the new feature changes/breaks a previous API (“new_feature”). This mechanism should always come with a due date.
  • a mechanism to test experimental features. Similar to the above but OFF by default. Experimental features are new features but which implementation does not cover the whole code, or which are not yet validated to be integrated in the code base (“experimental”)

TODO: add the deprecated mechanism + make sure we must set up dates when adding a new/deprecated feature + add the experimental feature

hugtalbot avatar Oct 27 '21 18:10 hugtalbot