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

Noisify genebank transform #36

Merged
merged 2 commits into from
May 20, 2024
Merged

Noisify genebank transform #36

merged 2 commits into from
May 20, 2024

Conversation

genehack
Copy link
Contributor

@genehack genehack commented May 20, 2024

Description of proposed changes

Adds expectation checks to transform-genbank-library that issue warnings when the input record does not have the expected shape. Also adds tests for both the default transformation behavior and the new warnings.

Related issue(s)

#35

Checklist

  • Checks pass
  • If adding a script, add an entry for it in the README.

genehack added 2 commits May 20, 2024 11:23
...about violated expectations around `database` and `location` fields
in the input record.

Note: I choose to have the `parse_location` method bail early and
return an unmodified record if the `location` field isn't found. The
other alternative is to issue the warning but add the empty `country`,
`division`, and `location` records. I don't have strong feelings about
this one way or the other; if somebody does, I'll change this
behavior.
@genehack genehack requested a review from joverlee521 May 20, 2024 18:24
Copy link
Contributor

@joverlee521 joverlee521 left a comment

Choose a reason for hiding this comment

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

I choose to have the parse_location method bail early and
return an unmodified record if the location field isn't found.

Sounds reasonable to me!

@genehack genehack merged commit 5b74edc into main May 20, 2024
2 checks passed
@genehack genehack deleted the noisify-genebank-transform-35 branch May 20, 2024 19:48
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