Avoid reaction id collisions in SplitReversibleReactionsTransform

[artgoldberg]

SplitReversibleReactionsTransform does

for submodel in model.submodels:
    for rxn in list(submodel.reactions):
        if rxn.reversible:
            # remove reversible reaction
            model.reactions.remove(rxn)
            submodel.reactions.remove(rxn)

            # create separate forward and reverse reactions
            rxn_for = submodel.reactions.create(
                model=model,
                id='{}_forward'.format(rxn.id),
                ...

The id of rxn_for could collide with the id of an existing reaction.
This problem can be avoided by

1. check whether id '{}_forward'.format(rxn.id) is already used by a reaction.
2. if not, use it.
3. if yes, create another id that does not collide. E.g., when copying file 'x' MacOS creates 'x copy', 'x copy 1', 'x copy 2', and so forth. It appears to use the first 'x copy n' available.

[johnsekar]

FYI, I think BNG solved this problem by using a _string prefix instead of suffix for automatically generated names. This minimizes chances for collisions as people do not usually provide names that begin with an underscore, and such names can additionally be forbidden when parsing user input.

…

On Tue, Nov 24, 2020, 23:22 Arthur P Goldberg ***@***.***> wrote: SplitReversibleReactionsTransform does for submodel in model.submodels: for rxn in list(submodel.reactions): if rxn.reversible: # remove reversible reaction model.reactions.remove(rxn) submodel.reactions.remove(rxn) # create separate forward and reverse reactions rxn_for = submodel.reactions.create( model=model, id='{}_forward'.format(rxn.id), ... The id of rxn_for could collide with the id of an existing reaction. This problem can be avoided by 1. check whether id '{}_forward'.format(rxn.id) is already used by a reaction. 2. if not, use it. 3. if yes, create another id that does not collide. E.g., when copying file 'x' MacOS creates 'x copy', 'x copy 1', 'x copy 2', and so forth. It appears to use the first 'x copy n' available. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#144>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMJHQJA7436TOI65VLZP3TSRSA6ZANCNFSM4UB2N3CA> .

[jonrkarr]

The approach John outlined (prefix or suffix) is commonly used in our field for issues like this.