ldmx-sw icon indicating copy to clipboard operation
ldmx-sw copied to clipboard

Harmonize standard python config variable names

Open bryngemark opened this issue 1 year ago • 3 comments

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.

bryngemark avatar May 16 '24 12:05 bryngemark

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_)

tvami avatar May 19 '24 14:05 tvami

@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.

bryngemark avatar May 23 '24 17:05 bryngemark

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.

tomeichlersmith avatar Jun 10 '24 13:06 tomeichlersmith