-
Notifications
You must be signed in to change notification settings - Fork 4
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
APS: duplicated affids removal #116
Conversation
tests/test_aps.py
Outdated
@@ -13,9 +13,11 @@ | |||
|
|||
from scrapy.http import TextResponse | |||
from hepcrawl.spiders import aps_spider | |||
from hepcrawl.parsers import aps_apr |
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.
what is this? 🤔
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.
A mistake, removed!
9dd8cde
to
666c8bf
Compare
8ebb7fa
to
2005e8b
Compare
tests/test_aps.py
Outdated
'surname': u'Sethna'}] | ||
|
||
sorted_expected_results = sorted( | ||
expected_results, key=lambda x: x['affiliations'][0]['value'] if x['affiliations'] else '') |
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.
@ErnestaP why do we have to sort the ‘expected_results’? Lists are maintaining the order, that means we just need to put the data in the correct order.
1a3c401
to
fb91246
Compare
fb91246
to
c41f697
Compare
@@ -64,6 +64,7 @@ | |||
{ | |||
"surname":"Alemi", | |||
"affiliationIds":[ | |||
"a1", |
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.
So, at the end we are not testing this anywhere, right? So no need to change it
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.
we are testing it, we see that parsed author does not have repetitive affiliations
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.
but where do we assert it?
No description provided.