[Core] [Properties] Remove MeshIndex from GetProperties

[miguelmaso]

📝 Description
closes #9538

Avoids a possible confusion in the python interface of ModelPart.GetProperties:

- GetProperties(prop_id, mesh_id) -> Properties instance
- GetProperties(mesh_id) -> Properties container
+ GetProperties(prop_id) -> Properties instance

🆕 Changelog

• Remove the MeshIndex from GetProperties, it will return a Properties instance with the given Id
• Cleanup add_model_part_to_python.cpp
• Update python scripts from apps and core
• Add a test

[miguelmaso]

tip: use this to find the behavior changes:

mmaso@PCCB197:~/Kratos/kratos$ grep -r "GetProperties([[:alnum:]_, ]\+)" --include "*.py"
tests/test_array_1d_interface.py:        prop = model_part.GetProperties(1,0)
tests/test_array_1d_interface.py:        prop = model_part.GetProperties(1,0)
tests/test_model_part.py:        self.assertEqual(model_part.GetProperties(0)[1].Id, 1)
python_scripts/read_materials_process.py:        model_part.GetProperties(prop_id).SetValue(CONSTITUTIVE_LAW, constitutive_law)
python_scripts/read_materials_process.py:        prop = model_part.GetProperties(property_id, mesh_id)
mmaso@PCCB197:~/Kratos/kratos$ 

[loumalouomega]

Okay from my side, but we did it that way because legacy reasons

[AlejandroCornejo]

On behalf of the @KratosMultiphysics/implementation-committee, We all agree with the implementation proposed, What do you guys think? @KratosMultiphysics/technical-committee ?

[ddiezrod]

ping @KratosMultiphysics/technical-committee

[pooyan-dadvand]

This is pure legacy and I fully agree with this change.

Nevertheless, I would put a visible message stating the change of behavior before changing it. Please consider that it may be used in the old context in some forks and changing the behavior would broke their code without any prompt.

[ddiezrod]

any advance on this?

[rubenzorrilla]

any advance on this?

I've just approved #9994 to start informing about this change.

In any case, I think we can safely merge this PR right away since it removes a legacy feature from the times with no submodelparts. Note that when we migrated to GitHub submodelparts were already there. Having said so, we can IMO assume that all (or the very vast majority) of our current forks are "submodelpart-based".

[loumalouomega]

any advance on this?

I've just approved #9994 to start informing about this change.

In any case, I think we can safely merge this PR right away since it removes a legacy feature from the times with no submodelparts. Note that when we migrated to GitHub submodelparts were already there. Having said so, we can IMO assume that all (or the very vast majority) of our current forks are "submodelpart-based".

Meanwhile some forks:

[loumalouomega]

any advance on this?

I've just approved #9994 to start informing about this change.

In any case, I think we can safely merge this PR right away since it removes a legacy feature from the times with no submodelparts. Note that when we migrated to GitHub submodelparts were already there. Having said so, we can IMO assume that all (or the very vast majority) of our current forks are "submodelpart-based".

Yes, and once the index is removed we can store the mesh directly and save some precious microseconds each time we do something in the mesh

[miguelmaso]

We can merge this on 1 September so that people can see the deprecation warning

[rubenzorrilla]

We can merge this on 1 September so that people can see the deprecation warning

Makes sense for me. 👍

[miguelmaso]

#10209 has reminded me that it's time to merge it