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 notifications@github.com 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 https://github.com/KarrLab/wc_lang/issues/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.