-
Notifications
You must be signed in to change notification settings - Fork 18
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
Fix: clean data accepted key #131
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #131 +/- ##
==========================================
+ Coverage 47.56% 57.05% +9.48%
==========================================
Files 7 7
Lines 473 482 +9
Branches 74 76 +2
==========================================
+ Hits 225 275 +50
+ Misses 244 202 -42
- Partials 4 5 +1 ☔ View full report in Codecov by Sentry. |
Fix: remove additional alias Fix: pynteny
680cc3a
to
114e8ef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this! Bugs being found and fixed in a place that makes sense to a reviewer.
Looks like some sort of test issue. |
Co-authored-by: Carol Willing <[email protected]>
yes! so essentially what i fixed here - there was a test that i didn't update. BUT i didn't see that it wasn't updated because it was not named correctly for pytest / vscode to find it when i ran the tests. So i just fixed that all by
thank you so much! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this contained the failing test. it wasn't running for me locally and i didn't notice that it wasn't running. this fixes that issue and ensures i see broken tests before pushing to GH thinking everything is green!
@@ -18,7 +18,7 @@ | |||
"archive": "[![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.8384174.svg)](https://doi.org/10.5281/zenodo.8384174)", | |||
"version_accepted": "5.1.1", | |||
"joss_doi": "[![DOI](https://joss.theoj.org/papers/10.21105/joss.01832/status.svg)](https://joss.theoj.org/papers/10.21105/joss.01832)", | |||
"date_accepted_(month/day/year)": "01/18/2024", | |||
"date_accepted": "01/18/2024", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was breaking the test because i fixed this key to ALWAYS be date_Accepted in this pr! so it needs to be fixed here too in the sample data.
all green ✅ merging! this one feels good as it feels like a good fix with a clean(er) fix implementation! |
closes #129
this fixes a bug in our workflow related to older issues with different formats for the date accepted key. it adds a helper clean function to the utils_clean module that looks for any key that begins with date_accepted and removes anything following it. this will simplify both our pydantic model ingest and also make it easy to add other scenarios if people modify that line.