Remove deprecation warning that may create errors

[pchaillo]

On my computer, these 3 lines create an error after my python update (I don't know why).

To be sure to avoid them for users, I just remove it.

What do you think ?

[EulalieCoevoet]

I would remove the lines instead of commenting them.

[damienmarchal]

Well, thank @pchaillo for the PR.

In the past the function was probably supposed to be used that way: index_from_axis(points=[something], axis=[anotherthing], old_indices = 'null')

Using a 'null' string instead of None to pass a None parameter was a bad idea it was probably fixed to replace it with None. To warn user of the API change, an extra steps was thus added to emit an exception so that calling points can update their code.

As the error message says it (badly):

DeprecationWarning("Attention, old_indices est à 'null', the new norm is old_indices = None")

To fix the error you need to stop pass 'null' in the old_indices parameters and use None.

If you want to make a PR:

• keep the lines that emit an errror, the fact it trigger in your code shows how usefull it is ;)
• propose an improved error message to help readers to understand better they have to fix the calling point

[damienmarchal]

I would remove the lines instead of commenting them. Eulalie is right, commenting code is a bad practice, always remove. But... on that specific problem neither commenting or removing is the proper fix :)

[EulalieCoevoet]

I still vote for a removal : it's been a year, plus the code has been added to the library with the deprecation warning. So I think it's safe to remove it.

But it would be interesting to understand why it triggers an error (and not just a warning as it's supposed to).

[damienmarchal]

@EulalieCoevoet

The observed behavior is what is expected to be.

The calling point is probably using the old convention passing "null" as parameter. This rise a DeprecationWarning. The exception, if not try-excepted, propagates through the call stack up to the top-level which is actually the SofaPython3 binding. At the place, an exception handler convert it into a Sofa.msg_error... which print an "error". This could be improved by replacing the use of msg_error for msg_deprecated when catching a child class of DeprecationWarning. We should make a PR to SofaPython3 to implement that.

And thus, depsite Sofa shows it as an Error instead of the Deprecation, the behavior is the one expected... reporting to users they are using the old API.

Small test...

import Sofa

def test(root=None):
    if root == "null":
        raise DeprecationWarning("Yo lo")
    root.addChild("This should work")

def createScene(root):
    root.addChild("First")
    test("null")
    root.addChild("Second")

Results in:

[ERROR]   [SofaPython3::SceneLoader] Unable to completely load the scene from file '/home/dmarchal/projects/dev/sofa1/build-clang/bin/test.py'.  
Python exception:   
  DeprecationWarning: Yo lo

At:
  /home/dmarchal/projects/dev/sofa1/build-clang/bin/test.py(5): test
  /home/dmarchal/projects/dev/sofa1/build-clang/bin/test.py(10): createScene

[EulalieCoevoet]

Okay I see.

[damienmarchal]

Okay I see.

Here is the improvement for Sofa... but it should probably be generalized to few others points and not only in the SceneLoader. https://github.com/sofa-framework/SofaPython3/pull/384