add the possibility to load only a mesh subset#20
add the possibility to load only a mesh subset#20jacklovell merged 3 commits intocherab:developmentfrom
Conversation
cherab/jet/machine/cad_files.py
Outdated
|
|
||
|
|
||
| def import_jet_mesh(world, override_material=None, tungsten_material=None, beryllium_material=None, | ||
| def import_jet_mesh(world, mesh_description=JET_MESH, override_material=None, tungsten_material=None, beryllium_material=None, |
There was a problem hiding this comment.
Please add this new argument to the end of the list. Otherwise existing codes which call this function with positional rather than keyword arguments will break.
There was a problem hiding this comment.
Good point, I shifted it to the end.
shift the new mesh_description to the end of the parameter list
cherab/jet/machine/cad_files.py
Outdated
| lambert_material=None, verbose=True): | ||
|
|
||
| for mesh_item in JET_MESH: | ||
| lambert_material=None, verbose=True, mesh_description=JET_MESH): |
There was a problem hiding this comment.
I don't think mesh_description is the best name for this argument. How about mesh_components instead?
There was a problem hiding this comment.
I'm not sure here. You have to pas a list of tuples containing path to the file and material. That is closer to description than components. What about component_description?
There was a problem hiding this comment.
I certainly think you want to pluralise whatever you choose, so that it's clear that this is a list of things you want to load. The docstring can describe what form is necessary for each element in components, but it should be reasonably clear to anybody reading code calling this function what is going on. And to me something like
component_description = VACUUM_VESSEL + DIAGNOSTICS + DIVERTOR_TILESmakes less sense than
mesh_components = VACCUM_VESSEL + DIAGNOSTICS + DIVERTOR_TILESThe latter just reads more naturally, at least to me as a native English speaker.
There was a problem hiding this comment.
Changed to mesh_components
|
I would like to ask for one small feature here. It would be very helpful if |
|
@vsnever I added the return list |
|
Thanks Matej! Looks good to me now. |
No description provided.