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

Add exclusion rules to mondo-excluded-values.yml #650

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matentzn
Copy link
Member

@matentzn matentzn commented Aug 28, 2024

Overview

First suggestion on how I would encode lexical rules for excluding synonyms altogether from synonym sync or externally managed content.

This PR:

  • One
  • Two
  • Three

Pre-merge checklist

Documentation

Was the documentation added/updated under docs/?

  • Yes
  • No, updates to the docs were not necessary after careful consideration

QC

Was the full pipeline run before submitting this PR using sh run.sh make build-mondo-ingest on this branch (after
docker pull obolibrary/odkfull:dev), and no errors occurred?

  • Yes
  • No, there are no functional (code-related) changes to the pipeline in the PR, so no re-run is necessary

New Packages

Were any new Python packages added?

Were any other non-Python packages added?

PR Review and Conversations Resolved

Has the PR been sufficiently reviewed by at least 1 team member of the Mondo Technical team and all threads resolved?

  • Yes

@joeflack4 joeflack4 added the enhancement New feature or request label Aug 28, 2024
Copy link
Contributor

@joeflack4 joeflack4 Aug 28, 2024

Choose a reason for hiding this comment

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

How would we like to use these rules?

Sorry I didn't bring up at the meeting; just thinking of these now.

  1. In cases of -confirmed or -updated, do we simply want (a) to add the EXCLUDE synonym type if it is missing? Or (b) not put these cases in the robot template, and instead output them into some other report file?
  2. In -added cases, do we want to (a) add the synonym and add EXCLUDE? or (b) not put these cases in the robot template, and instead output them into some other report file?
  3. Is there more we'd like to do w/ these rules?

Copy link
Contributor

Choose a reason for hiding this comment

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

When would we like to start using this in the synonym sync?

@@ -1,4 +1,9 @@
exclusions:
synonym_exclusion_rules:
- rule: ".*otherwise specified"
Copy link
Contributor

Choose a reason for hiding this comment

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

Regex-based lexical exclusion rules

I'm fine w/ this implementation, and with just adding rules incrementally as we go, starting with these.

Base automatically changed from develop to main November 24, 2024 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants