Skip to content

Week 3#3

Merged
LYang-uva merged 9 commits into
mainfrom
week-3
May 27, 2026
Merged

Week 3#3
LYang-uva merged 9 commits into
mainfrom
week-3

Conversation

@LYang-uva

Copy link
Copy Markdown
Collaborator

No description provided.

@magdalena-grubesa magdalena-grubesa 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.

  • Can the package be installed? Yes, I was able to install it in a virtual environment
  • Is the vignette set up correctly? Yes, it contains all the necessary information and good examples.
  • Did the unit tests run? yes
  • Did they pass? yes

@erwardenaar erwardenaar 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.

  • Can the package be installed?

Yes, cleanly. One note: the README clone instructions point to main, so a user following them gets the old stub version, not the week-3 code. Worth noting the branch or merging.

  • Is the vignette set up correctly?

Yes, it is well structured, covers purpose, users, workflow, and examples. The dependency table in the vignette is actually more complete than the README (it includes sentence-transformers, benepar, etc.), so it might be worth syncing the README to match.

  • Did the unit tests run? Did they pass?

All 31 pass. The tests themselves are great: 5 files, proper mocking, fixtures, edge cases, error paths for parsers and scorers.

@LYang-uva LYang-uva merged commit a366a9f into main May 27, 2026
1 check passed
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.

3 participants