inspirehep / refextract

Extract bibliographic references from (High-Energy Physics) articles.
GNU General Public License v2.0
130 stars 30 forks source link

kbs: update report-nbumbers.kb #66

Open fschwenn opened 5 years ago

fschwenn commented 5 years ago

Signed-off-by: fschwenn florian.schwennsen@desy.de

tsgit commented 5 years ago

I think it would be good to have a sample original reference line for each pattern, or at least each pattern group as a list of test cases. This would confirm the pattern does what is intended to do and no other pattern matches. At least would show when hyphens are incorrect. e.g. similar to https://github.com/inspirehep/refextract/blob/master/tests/test_tag.py#L132-L135 with tag_report (which you'd have to define) instead of tag_arxiv or patterned after https://github.com/inspirehep/refextract/blob/master/tests/test_engine.py#L102-L118 and elsewhere in that test file. instead of multiple report_numbers I would focus on one at a time, or one pattern type at a time

tsgit commented 5 years ago

@fschwenn take a look at https://github.com/inspirehep/refextract/pull/67/files there you see test-cases Melissa added for the Fermilab report number parsing

also, since that PR was merged, you need to rebase your PR

fschwenn commented 5 years ago

Dear Thorsten,

I think I can add test routines following Melissas example. Thanks for pointing them out to me. But I will probably fail with the rebase stuff. Would it be ok for you to just reject my previous pull request and I implement my additions on top of the actual master (in a new branch)? I know that in principle git allows much more elegant ways to handle it.

Best regards, Florian

-- Florian Schwennsen Deutsches Elektronen-Synchrotron DESY Building 01 Room O1.446 phone: +49-40-8998-6190

From: "Thorsten Schwander" notifications@github.com To: "inspirehep/refextract" refextract@noreply.github.com Cc: "Florian Schwennsen" florian.schwennsen@desy.de, "Mention" mention@noreply.github.com Sent: Tuesday, 11 June, 2019 19:15:13 Subject: Re: [inspirehep/refextract] kbs: update report-nbumbers.kb (#66)

[ https://github.com/fschwenn | @fschwenn ] take a look at [ https://github.com/inspirehep/refextract/pull/67/files | https://github.com/inspirehep/refextract/pull/67/files ] there you see test-cases Melissa added for the Fermilab report number parsing

also, since that PR was merged, you need to rebase your PR

— You are receiving this because you were mentioned. Reply to this email directly, [ https://github.com/inspirehep/refextract/pull/66?email_source=notifications&email_token=ABZ5NE6F6G4HACBJHRCZLKLPZ7MSDA5CNFSM4HO3KUV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXN3TEQ#issuecomment-500939154 | view it on GitHub ] , or [ https://github.com/notifications/unsubscribe-auth/ABZ5NE4WMMWBML7RHAKJTZDPZ7MSDANCNFSM4HO3KUVQ | mute the thread ] .