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

Prioritize pmc json #1101

Merged
merged 5 commits into from
Apr 17, 2020
Merged

Prioritize pmc json #1101

merged 5 commits into from
Apr 17, 2020

Conversation

lukuang
Copy link
Contributor

@lukuang lukuang commented Apr 17, 2020

According to the FAQs of the CORD-19 data set, we should use pmc json whenever possible, which were made available in recent releases.

@codecov
Copy link

codecov bot commented Apr 17, 2020

Codecov Report

Merging #1101 into master will decrease coverage by 0.06%.
The diff coverage is 37.50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1101      +/-   ##
============================================
- Coverage     46.61%   46.55%   -0.07%     
  Complexity      680      680              
============================================
  Files           143      143              
  Lines          8233     8242       +9     
  Branches       1168     1172       +4     
============================================
- Hits           3838     3837       -1     
- Misses         4056     4065       +9     
- Partials        339      340       +1     
Impacted Files Coverage Δ Complexity Δ
...o/anserini/collection/CovidCollectionDocument.java 38.70% <33.33%> (-7.45%) 5.00 <1.00> (ø)
.../anserini/collection/CovidParagraphCollection.java 83.33% <42.85%> (-6.38%) 3.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 115958b...91912a0. Read the comment docs.

@lintool
Copy link
Member

lintool commented Apr 17, 2020

hi @lukuang just for completeness, can we keep the pdf_json copy?
@edwinzhng can you take a look please?

@lintool lintool requested a review from edwinzhng April 17, 2020 15:55
Copy link
Member

@edwinzhng edwinzhng left a comment

Choose a reason for hiding this comment

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

Thanks for fixing! LGTM, ran it locally and it works

@lintool
Copy link
Member

lintool commented Apr 17, 2020

Waiting for @lukuang to revert the pdf_json copy and then I'll merge.

Copy link
Contributor Author

@lukuang lukuang left a comment

Choose a reason for hiding this comment

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

Waiting for @lukuang to revert the pdf_json copy and then I'll merge.

Done. Please go ahead and ship.

@lintool lintool merged commit cc3b0a3 into castorini:master Apr 17, 2020
@lintool
Copy link
Member

lintool commented Apr 17, 2020

Thanks @lukuang !

crystina-z pushed a commit to crystina-z/anserini that referenced this pull request Oct 28, 2022
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.

3 participants