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

Updated the claims code #49

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

97harsh
Copy link
Contributor

@97harsh 97harsh commented Jul 2, 2020

made the code shorter, removed the output component, which was making the notebook so huge

Relevant notebook can be viewed at: https://github.com/97harsh/task-vt/blob/claim_extraction/contradictory_claims/notebooks/2T.4.5%20Claim%20extraction.ipynb

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

Review Jupyter notebook visual diffs & provide feedback on notebooks.


Powered by ReviewNB

@cthoyt
Copy link
Member

cthoyt commented Jul 2, 2020

Is there any chance you can add some of the code in these notebooks as python modules that are imported in the notebooks? This has several benefits:

  1. allows for code quality checks
  2. Allows for code review in this PR
  3. Makes notebooks more reproducible
  4. Removes clutter from notebook so the notebooks can be focused on the scientific story rather than the code that accomplishes it

@97harsh
Copy link
Contributor Author

97harsh commented Jul 3, 2020

Is there any chance you can add some of the code in these notebooks as python modules that are imported in the notebooks? This has several benefits:

  1. allows for code quality checks
  2. Allows for code review in this PR
  3. Makes notebooks more reproducible
  4. Removes clutter from notebook so the notebooks can be focused on the scientific story rather than the code that accomplishes it

There is not much to modularise,
the only thing that this notebook does is implements a this allennlp module on the Cord dataset to generate the Sentence claims in sentence
had pushed it again, because the earlier output had a lot of clutter

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.

None yet

2 participants