RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.27k stars 1.26k forks source link

[multibody] `model_directives.h` (only) uses namespace `multibody::parsing` #17385

Open RussTedrake opened 2 years ago

RussTedrake commented 2 years ago

model_directives.h, process_model_directives.h, and scoped_names.h stand out as the only headers in multibody/parsing that use the namespace multibody::parsing. All other headers/classes in this directory simply use the namespace multibody. This causes small but unnecessary friction when having to remember/check the namespace for the various parsing methods.

Expected: these files should use namespace multibody only, like the other headers/classes in this directory.

cc @ggould-tri as the original author

jwnimmer-tri commented 2 years ago

I would buy the main entry points LoadModelDirectives and ProcessModelDirectives moving into the simpler namespace, but I think many of the helper types in other files (e.g., AddWeld or PrefixName) are named too generically to live in multibody as-is. For those names, the clarity that they are the parsing flavor of the name is important. We could rename them as part of the move (so that they could stand on their own), but I'm not sure it's worth the effort. I assume the major point of friction is only with the entry point?

RussTedrake commented 2 years ago

Yes. Moving only the entry points would resolve my concern.

jwnimmer-tri commented 1 year ago

Now that dmd.yaml can go in through the Parser API directly, my bid would be to investigate deprecating process_model_directives.h, instead of changing its namespace.

That's probably too aggressive, so perhaps more subtly: try to figure out why people are still using process_model_directives.h, and help them port to something better.