♻️ Refactoring Sample by introducing features module

[bstaber]

This PR tries to simplify a bit the sample.py module by splitting responsabilities.

Summary:

• A features.py module that contains SampleScalars and SampleMeshes
• Those new objects handle the scalars and mesh mechanics
• Several methods are removed from Sample

Checklist

• Typing enforced
• Documentation updated
• Changelog updated
• Tests and Example updates
• Coverage should be 100%

[gitnotebooks]

Review these changes at https://app.gitnotebooks.com/PLAID-lib/plaid/pull/171

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[bstaber]

I think it's ready, you might want to check if I left all the methods the user needs in Sample.

I put SampleScalars in containers/scalars.py and SampleMeshes in containers/meshes.py.

[xroynard]

maybe we should add an entry to the Changelog, to warn users that it is a major refactoring and that it might introduce problems

[xroynard]

I put SampleScalars in containers/scalars.py and SampleMeshes in containers/meshes.py.

I think maybe scalars.py and meshes.py should be gathered in a sub-package of plaid.containers, for exemple plaid.containers.features

What do you think about that ?

[bstaber]

@xroynard it's done let me know what you think about it

[bstaber]

Methods only in Sample

• get_features_from_identifiers
• get_all_features_identifiers_by_type
• del_scalar
• get_scalar_names
• from_features_identifier
• update_features_from_identifier
• add_time_series
• merge_features
• serialize_model
• del_time_series
• set_default_base
• get_feature_from_string_identifier
• set_default_time
• get_all_features_identifiers
• get_time_series
• add_scalar
• set_default_zone_base
• get_scalar
• get_feature_from_identifier
• del_all_fields
• link_tree
• get_time_series_names

---

Methods only in SampleMeshes

• get_elements
• get_zone_assignment
• set_vertices
• get_links
• has_zone
• add_tree
• get_points
• get_vertices
• get_zone_type
• get_topological_dim
• get_nodal_tags
• get_zone_names
• get_base
• get_all_mesh_times
• del_zone
• set_meshes
• set_points
• del_tree
• get_base_assignment
• get_time_assignment
• get_zone
• get_base_names
• get_physical_dim
• del_base
• init_tree
• has_base

[bstaber]

Do you want to keep link_tree in Sample btw?

[casenave]

Do you want to keep link_tree in Sample btw?

we can remove, but the temporal dataset will not be working anymore, to discuss when to do it

[casenave]

We can discuss this in person

[casenave]

Do you want to keep link_tree in Sample btw?

We can put in in an experimental module and get rid of it in the main package.

[casenave]

I think we can merge (after CHANGELOG is updated), and keep the experimental folder for another PR ? What is your take on this @bstaber @xroynard ?

[bstaber]

Do you want me first to:

• rename the underscores in _meshes and _scalars first ?
• put a method add_mesh in Sample? I think all the methods related to meshes are in SampleMeshes right now (just checking)

[xroynard]

Do you want me first to:

• rename the underscores in _meshes and _scalars first ?
• put a method add_mesh in Sample? I think all the methods related to meshes are in SampleMeshes right now (just checking)

there will already be a method add_mesh after #191 is merged

[casenave]

Do you want me first to:

• rename the underscores in _meshes and _scalars first ?
• put a method add_mesh in Sample? I think all the methods related to meshes are in SampleMeshes right now (just checking)

we can rename in another PR if you want

[bstaber]

Ok let's go then, I think I need a new approval

[casenave]

Ok let's go then, I think I need a new approval

oops there still is the correction to the CHANGELOG, and some conflicts appear :-(

[bstaber]

LGTR 🥵

[bstaber]

[casenave]

Looks nice to me, thanks ! Just mentioning that the meshes arg may be renamed lie time_data or something else when/if we succeed in including a global in the CGNS tree to replace scalar and time_series.

[bstaber]

FYI: tried to remove the underscores but it's not that simple. I have to change the way we use pydantic in Sample for that, I prefer to leave it for another PR