Skip to content

Improve GlycoCT parser coverage - #24

Merged
fubin1999 merged 3 commits into
mainfrom
fix-glycoct
Jul 1, 2026
Merged

Improve GlycoCT parser coverage#24
fubin1999 merged 3 commits into
mainfrom
fix-glycoct

Conversation

@fubin1999

@fubin1999 fubin1999 commented Jul 1, 2026

Copy link
Copy Markdown
Member

Summary

Improve parse_glycoct() coverage for GlycoCT records from the sequence corpus.

Details

  • Accept space-separated GlycoCT records, including CSV-export style records where RES, residues, LIN, and linkages are stored on one line.
  • Map representable generic GlycoCT descriptors to glyrepr monosaccharides, including generic HEX, N-acetylated HEX, deoxy-HEX, and generic Neu5Ac/Neu5Gc descriptors.
  • Map direct GlycoCT n-sulfate substituents to N-sulfated amino sugars.
  • Add focused regression coverage for the newly supported parser cases.
  • Update NEWS.md with the live (#24) suffix for the GlycoCT entries.

Verification

  • git rebase main
  • Rscript -e 'devtools::document()'
  • Rscript -e 'devtools::load_all(quiet = TRUE); devtools::test(filter = "parse-glycoct")' -> 306 passed
  • Rscript -e 'devtools::test()' -> 943 passed
  • air format R/parse-glycoct.R tests/testthat/test-parse-glycoct.R
  • git diff --check main..HEAD

Copilot AI review requested due to automatic review settings July 1, 2026 02:21
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR expands parse_glycoct() to handle additional real-world GlycoCT encodings found in the sequence corpus, improving successful parsing and adding regression coverage for the new cases.

Changes:

  • Added preprocessing to accept space-separated / single-line GlycoCT records (e.g., CSV-export style).
  • Added mapping for representable generic GlycoCT descriptors (generic HEX, N-acetylated HEX, deoxy-HEX, and generic Neu5Ac/Neu5Gc backbones).
  • Added mapping of direct GlycoCT n-sulfate substituents to N-sulfated amino sugars, plus focused tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/testthat/test-parse-glycoct.R Adds targeted regression tests for space-separated records and newly supported generic/N-sulfate mappings.
R/parse-glycoct.R Implements line splitting for single-line records and adds generic composite matching + helper predicates/mappers.
NEWS.md Documents the newly supported GlycoCT parsing/mapping behaviors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@fubin1999

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d85665b007

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread R/parse-glycoct.R Outdated
Comment thread R/parse-glycoct.R Outdated
@fubin1999
fubin1999 merged commit 74f4364 into main Jul 1, 2026
13 checks passed
@fubin1999
fubin1999 deleted the fix-glycoct branch July 1, 2026 04:10
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