-
Notifications
You must be signed in to change notification settings - Fork 0
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 AM dataset - CDCP #15
Conversation
e6bd22b
to
af47a5e
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.
early feedback
EDIT: Please also remove dataset_builders/hf/cdcp
again, this is not so important for now. But please still create the Huggingface dataset card for DFKI-SLT/cdcp at the Huggingface Hub.
158af3b
to
536e9f4
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.
Some more feedback. There are still several files in the diff that shouldn't be listed there:
- the
poetry.lock
should now be again the original - please remove the
__init__.py
files anywhere indataset_builders
document/types.py
: most of the classes are not used at all. For classes, that are just used in the tests, define them just there.
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.
Some ideas for improvement and how-to increase test coverage
Background:
- the test coverage should be as high as possible
- source code lines that are new in this PR, but not covered by any test execution are marked by the Codecov Bot (box with yellow and named "Codecov / codecov/patch") in the diff view
536e9f4
to
ceef7cd
Compare
Codecov Report
@@ Coverage Diff @@
## main #15 +/- ##
==========================================
+ Coverage 92.53% 92.93% +0.39%
==========================================
Files 16 17 +1
Lines 1206 1274 +68
==========================================
+ Hits 1116 1184 +68
Misses 90 90
📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
ceef7cd
to
f7b3753
Compare
f7b3753
to
213894d
Compare
7768b30
to
77baea5
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.
This is a review just for the code for now. I will review the readmes separately.
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.
Feedback for the readmes.
00b9c77
to
4f74a05
Compare
As described in #10, we work on CDCP, an argumentation mining dataset.
Specifically for the CDCP dataset, we add:
pie/cdcp.py
HF/README.md
andpie/README.md
files. EDIT: the HF dataset cards lives heretests/dataset_builders/pie/test_cdcp.py