Kratos
Kratos copied to clipboard
[Core] [Properties] Remove MeshIndex from GetProperties
📝 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
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$
On behalf of the @KratosMultiphysics/implementation-committee, We all agree with the implementation proposed, What do you guys think? @KratosMultiphysics/technical-committee ?
ping @KratosMultiphysics/technical-committee
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.
any advance on this?
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".
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:

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
We can merge this on 1 September so that people can see the deprecation warning
We can merge this on 1 September so that people can see the deprecation warning
Makes sense for me. :+1:
#10209 has reminded me that it's time to merge it