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

[EM] Don't concatenate different nodes by default #1595

Merged
merged 3 commits into from Oct 9, 2018

Conversation

Projects
None yet
2 participants
@zuphilip
Copy link
Collaborator

zuphilip commented Mar 17, 2018

This lead to title and abstracts containing also a
translated form, see examples in Cairn.info.js.

@zuphilip zuphilip added the Workshop label Mar 17, 2018

@zuphilip zuphilip force-pushed the zuphilip:no-concatenate-em branch from e991411 to 2afbf55 Sep 1, 2018

@zuphilip zuphilip requested a review from adam3smith Sep 1, 2018

zuphilip added some commits Mar 17, 2018

[EM] Don't concatenate different nodes by default
This lead to title and abstracts containing also a
translated form, see examples in Cairn.info.js.
Update tests, change handling of og:site
- The publicationTitle is also considered for cleaning title
when this information comes from doc.title. However, after
e7493f3 this information was not always given, e.g. test case
http://volokh.com/2013/12/22/northwestern-cant-quit-asa-boycott-member/
Therefore, we switch here the condition but left the field the same.
- DIVA is the site name in og:site and not the proceeding title.
- The salon website has moved most of its meta tags into the body
and therfore the recognition is quite bad.
- The detection of Hindawi test case still fails, but maybe the can
fix this...

@zuphilip zuphilip force-pushed the zuphilip:no-concatenate-em branch from 2afbf55 to 2838e0f Oct 7, 2018

@zuphilip

This comment has been minimized.

Copy link
Collaborator Author

zuphilip commented Oct 7, 2018

@adam3smith Can you review these changes please?

I have rebased this PR now and also fixed some unwanted consequences from the previous EM changes.

@@ -1383,12 +1366,12 @@ var testCases = [
}
],
"date": "2013",
"abstractNote": "Signaling data from the cellular networks can provide a means of analyzing the efficiency of a deployed transportation system and assisting in the formulation of transport models to predict its fut ...",
"abstractNote": "DiVA portal is a finding tool for research publications and student theses written at the following 47 universities and research institutions.",

This comment has been minimized.

Copy link
@adam3smith

adam3smith Oct 9, 2018

Collaborator

why is it picking up the site description from og:description rather than the abstract from DC.description? Since this is also true for the current EM translator, I'll accept this and create a new issue.

This comment has been minimized.

Copy link
@adam3smith

@adam3smith adam3smith merged commit 3ebf896 into zotero:master Oct 9, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@adam3smith

This comment has been minimized.

Copy link
Collaborator

adam3smith commented Oct 9, 2018

Thanks, also for the EM fix, not sure why that didn't show up in testing for me. Only thing I've changed is to add a comment for the allValues variable for ISSN/ISBN. See also the new ticket above -- something weird happening with abstracts.

@zuphilip zuphilip deleted the zuphilip:no-concatenate-em branch Oct 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.