LDMX-Software / ldmx-sw

The Light Dark Matter eXperiment simulation and reconstruction framework.
https://ldmx-software.github.io
GNU General Public License v3.0
20 stars 15 forks source link

Harmonize standard python config variable names #1354

Open bryngemark opened 1 month ago

bryngemark commented 1 month ago

Is your feature request related to a problem? Please describe. Ex. I'm always frustrated when [...] ... I try to set something basic like the input collection or input pass name to use in my config on the fly, and I have to look up what exact naming scheme that particular producer uses for it. input_pass_name, digi_pass_name, input_hit_pass_name, or maybe inputPassName?

Describe the solution you'd like I would like to have one standard way of defining this type of basic variable that almost every producer uses, and just harmonize it.

Describe alternatives you've considered I'm wondering if there are good examples of when we need multiple input collections (and associated passes)? I think the EcalRecHitProducer is one of the very few I've encountered where there are multiple and I suppose we should have a clever solution there. Alternatively, we'd live with looking up the proper name for these few cases.

tvami commented 1 month ago

If I look at pass_name in the python files: https://github.com/search?q=repo%3ALDMX-Software%2Fldmx-sw+pass_name++language%3APython&type=code

it seems it's mostly the input_pass_name that's used, and then inputPassName is used in the TS. Based on this I'd say we should go with input_pass_name to minimize work on the transition.

I think generally it would be good to have some guidelines on the code/naming style. Like in the cxx the same variable should be called with camel, i.e. having the inputPassName. And then also enforce that the global variables have the trailing underscore (so maybe that example is actually inputPassName_)

bryngemark commented 1 month ago

@tvami i think these are good suggestions, and if we find a general principle, it will be easier for streamlining input and output collection variable names etc too.

tomeichlersmith commented 3 weeks ago

While it would be a big lift, snake_case is actually what the Google C++ style guide suggests for variable names. (I'm guessing clang-format makes the choice to not check the case of variable names when formatting which is why this has been missed.) This would imply that we should change the C++ names to be snake case and then the Python config names and the C++ member variables would only differ by a trailing underscore.

Edit: clang-format limits itself to local changes that are irrelevant to the compiler, so it does not check for variable names (among other things). This makes its Google style a subset of the Google style guide linked above.