Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add rxnMAIDfield to JSON file #175

Merged
merged 3 commits into from
Jun 2, 2020
Merged

feat: add rxnMAIDfield to JSON file #175

merged 3 commits into from
Jun 2, 2020

Conversation

haowang-bioinfo
Copy link
Member

Main improvements in this PR:

I hereby confirm that I have:

  • Tested my code on my own computer for running the model
  • Selected devel as a target branch

- Since the addition of `spontaneous` field, there could be non-string elements in array. This adjustment allows the recognition of last element regardless of its type.
The change of this commit is made through following command:
`rxnAssoc.rxnMAID = regexprep(rxnAssoc.rxns,'^HMR_','MA_');`
@JonathanRob
Copy link
Collaborator

Two comments/suggestions:

  1. Maybe leave the rxnMAID field blank for reactions that do not have an ID of the form MA_####, instead of just using their original ID? I realize it's temporary and probably doesn't matter, but this would at least keep all of the IDs in the rxnMAID field consistent.

  2. Consider adding an extra digit, since we're already surpassing 10,000 reactions; e.g., MA_3905 would be MA_03905.

1. Remove ids that do not start with `MA_` prefix;
2. Pad in zero to digital part to ensure the elements with identical length.
@haowang-bioinfo
Copy link
Member Author

@JonathanRob the elements are reformatted as suggested.

@haowang-bioinfo haowang-bioinfo merged commit e8d4799 into devel Jun 2, 2020
@haowang-bioinfo haowang-bioinfo deleted the feat/rxnMAID branch June 2, 2020 17:58
@haowang-bioinfo haowang-bioinfo mentioned this pull request Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants