-
Notifications
You must be signed in to change notification settings - Fork 7
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
Implement a basic set of node and edge errors #204
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #204 +/- ##
==========================================
+ Coverage 95.00% 95.45% +0.45%
==========================================
Files 18 20 +2
Lines 921 1013 +92
==========================================
+ Hits 875 967 +92
Misses 46 46 ☔ View full report in Codecov by Sentry. |
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.
Generally looks great, just left a few comments on the code. The tests are so sensible and complete 😄 I love it. One potential problem is that I tried to look at the docs and it seems that the build is not working in the PR? Not sure if that is just a one-time issue or if there is something actually wrong, but if you could investigate that would also be great.
Closes #181 by implementing a set of node and edge errors outside of the CTC errors. Currently, these errors only support one to one matching, but they could be extended to support many-to-many in the future if desired. I updated the metrics and docs accordingly.
FYI, I'm open to a name other than BasicErrors and BasicMetrics, but couldn't come up with anything better 😅
For the documentation of the metrics, I kept it pretty minimal and assumed that people know/can google precision/recall/f1, but if you think I should elaborate more let me know.
Types of Changes
What types of changes does your code introduce? Delete those that do not apply.
Which topics does your change affect? Delete those that do not apply.
Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.