TeamCOMPAS / COMPAS

COMPAS rapid binary population synthesis code
http://compas.science
MIT License
65 stars 66 forks source link

Change text appended to column headers in output files: improve clarity #342

Closed jeffriley closed 4 years ago

jeffriley commented 4 years ago

Is your feature request related to a problem? Please describe.

Each variable written to an output file has an associate header string. If the variable can be associate with a single star then it might have "_1", "_2", "_SN", or "_CP" appended - to indicate that the variable refers to the primary star, secondary star, the supernova star, or the companion (to the supernova).

Sometimes that can be confusing, especially for the variable whose header strings being with "SN_" - because the header strings might then be "SN_Type_SN", which isn't very clear.

We have to have a mechanism for differentiating between the four contexts above (primary, secondary, supernova, and companion). It may not make a lot of sense, but someone might, for example, print the "SN_Type" for both stars in the record for the Supernovae output file, and in that case we would (as our code stands at the moment) get column headers "SN_Type_SN", and "SN_Type_CP", which I guess look a little odd. It doesn't matter why anyone would print both values on the same record - we need to cater for it just because they can do it.

Describe the solution you'd like

Proposal:

As a reasonably simple (somewhat mitigating) fix, I suggest we change to text appended to the header strings from "_1", "_2", "_SN", and "_CP" to "(1)", "(2)", "(SN)", and "(CP)".

That would mean instead of the (current) headers

"Mass_1", "Mass_2", "Metallicity_1", "Metallicity_2"

we would have

"Mass(1)", "Mass(2)", "Metallicity(1)", and "Metallicity(2)"

And in the example above, instead of

"SN_Type_SN" and "SN_Type_CP"

we would have

"SN_Type(SN)" and "SN_Type(CP)".

(note that we can't have spaces in the header strings - it causes problems for parsing)

I know it's not a huge difference, but I think it is a little bit clearer.

Describe alternatives you've considered There aren't a lot of options that I can think of - happy to take suggestions.

Additional context Add any other context or screenshots about the feature request here.

jeffriley commented 4 years ago

Unless there are any objections, I will implement this change some time this week.

ilyamandel commented 4 years ago

No objections from me.

ilyamandel commented 4 years ago

Thank you for the fix, @jeffriley ! Closing.