Canonicalisation of Reactions from ReactionEquation

[A-Thakkar]

Related to https://github.com/rxn4chemistry/rxn-chemutils/issues/8

proposal to add canonicalisation and removal of atom mapping as arguments to the to_string method of ReactionEquation
reaction.to_string(canonicalise=True, remove_atom_mapping=True)

[avaucher]

My personal choice would be not to do that. A few points of explanation:

• I see the conversion to string, the canonicalization, and the removal of atom mapping as three distinct things. To me the to_string function should really be about that: the conversion to a string. [PS1]
• This would raise the question of adding it to all the reaction-to-string conversions, i.e. also the one with extended SMILES
• I try to avoid confusion by keeping the number of arguments small
• Linked to the last point: this may lead to other potential arguments such as sorting, removal of duplicates, etc.

Having said that, I am more open to having an external function doing multiple of those if needed. Something along the lines of cleanup_reaction(reaction_equation)? Here I would prefer to work on the reaction equation as on the str, as it is compatible with extended SMILES for instance. Also, I tend to prefer free non-member functions when possible.

What are your thoughts on this?

[PS1]: following up on the arguments with free functions and other kinds of strings, I was already tempted a few times to remove to_string from the ReactionEquation class 😉