Skip to content

Regex fixes for compound name parsing#13

Closed
lasagen wants to merge 6 commits into
Robaina:masterfrom
lasagen:master
Closed

Regex fixes for compound name parsing#13
lasagen wants to merge 6 commits into
Robaina:masterfrom
lasagen:master

Conversation

@lasagen

@lasagen lasagen commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

Hello! Thank you so much for your work on BRENDApyrser. I was using it for a study and found a few issues.

  1. Splitting on the "+" character to parse out the reactions was also getting rid of +s indicating that a compound was a positive ion, e.g. H+. I changed substratesAndProducts to only split on "+" if there are spaces around it and leave +s in compound names.
  2. Comments inside pipe | characters were being left in the compound name instead of being added to the meta field. I added them to the meta field in __extractDataField and __extractReactionMechanismInfo.
  3. Issue Compound names wrongly parsed #12 mentions compound names being wrongly parsed when they contain parentheses. This is because the parser was extracting the first match of something in parentheses and putting that in the meta field. I changed extractDataLineInfo to only add something in parentheses to the meta field if it starts with whitespace.

These worked for me, but there are definitely better ways to do them, so feel free to suggest changes. Thanks again!

@lasagen

lasagen commented Dec 29, 2025

Copy link
Copy Markdown
Contributor Author

Hey, I saw that the macos-latest check was failing for this PR - looking at the details it seems to be failing in the middle of setting up the conda environment, before the build and test stages. Is this just a case where you have to update the action, or is there something I'm missing that I should change?

Robaina added a commit that referenced this pull request Jun 14, 2026
BRENDA's JSON still embeds legacy annotations inside reaction-string
values. Splitting reaction sides on a bare "+" dropped ion charges
(H+ -> H, NAD+ -> NAD), and |...| pipe comments and <ref> tags leaked
into compound names.

- add _clean_reaction_string(): strips |...| comments, {...} reversibility
  and numeric <n,n> reference tags, preserving chemical notation such as
  glycosidic "(1->4)" bonds and polymer subscripts ([...]n+m)
- split reaction sides on " + " so ionic charges and polymer "+" stay
  attached to the compound name (fixes filter_by_substrate/product/compound
  for ions, e.g. "NAD+")
- fold substrates_products pipe comments into `comment` instead of
  discarding them

Re-addresses the parsing issues reported in #13 against the JSON parser
(ion-charge loss and |...| comment leakage). The parentheses issue (#12)
is already handled by the structured JSON `comment` field.

Co-authored-by: Clark Pohl <clarklettuce@gmail.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Robaina

Robaina commented Jun 14, 2026

Copy link
Copy Markdown
Owner

Hi @lasagen, thank you for these fixes and the detailed write-up! 🙏 Since you opened this, BRENDA switched from the flat text file to a structured JSON document (schema 2.0.0), so the parser was rewritten on and these regex changes no longer apply to the new code. But two of the three issues you found were still present in the JSON parser, and I've re-implemented your fixes there:

Ion charges (H+, NAD+) were still dropped — fixed by splitting on " + " (your approach).
|...| pipe comments still leaked into compound names — now stripped and folded into the comment field.
The parentheses issue (#12) is now handled by the JSON's separate comment field.
I credited you as co-author on commit 849e8cb so your contribution stays in the history. Closing in favour of the JSON rewrite, but thank you, these were real bugs and your report directly improved the new parser!

@Robaina Robaina closed this Jun 14, 2026
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