Skip to content
Please note that GitHub no longer supports your web browser.

We recommend upgrading to the latest Google Chrome or Firefox.

Learn more
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

Update Springer Books + Springer Link #2031

Merged
merged 5 commits into from Oct 22, 2019

Conversation

@zuphilip
Copy link
Collaborator

commented Oct 19, 2019

No description provided.

Copy link
Collaborator

left a comment

One small request. Everything else looks good to merge

@@ -148,6 +148,11 @@ function scrapeBook(doc, url) {
var edition = ZU.xpathText(doc, '//dt[text()="Edition Number" or text()="Auflage"]/following-sibling::dd[1]');
if (edition && edition !== "1") item.edition = edition;

var doi = ZU.xpathText(doc, '//dt[text()="DOI"]/following-sibling::dd[1]');
if (doi) {
item.extra = "DOI: " + doi;

This comment has been minimized.

Copy link
@adam3smith

adam3smith Oct 20, 2019

Collaborator

Since embedded metadata can write unknown fields to Extra as welll, could we add a test for content in Extra here to make sure we're not overwriting anything?
Along these line

if (item.extra) {
 if (!item.extra.includes("DOI") {
   item.extra += "\nDOI: " + doi;
}
else {
 item.extra = "DOI: " + doi;
}

This comment has been minimized.

Copy link
@zuphilip

zuphilip Oct 21, 2019

Author Collaborator

(+ } before the else)

RIS.js Outdated
@@ -405,6 +404,7 @@ var degenerateImportFieldMap = {
CA: "unsupported/Caption",
CR: "rights",
CT: "title",
CY: "place",

This comment has been minimized.

Copy link
@adam3smith

adam3smith Oct 20, 2019

Collaborator

Actually, this doesn't seem right, either. CY is already included for place, just not for conference papers see

CY: {

This comment has been minimized.

Copy link
@zuphilip

zuphilip Oct 20, 2019

Author Collaborator

Not exporting is okay, but why should we not import it if it is there? (There are similar cases in the RIS translator already e.g. for NV.)

Example:

TY  - CONF
AU  - Rusch, Anja
AU  - Wille, Rudolf
ED  - Bock, Hans-Hermann
ED  - Polasek, Wolfgang
PY  - 1996
DA  - 1996//
TI  - Knowledge Spaces and Formal Concept Analysis
BT  - Data Analysis and Information Systems
SP  - 427
EP  - 436
PB  - Springer Berlin Heidelberg
CY  - Berlin, Heidelberg
AB  - J.-P. Doignon and J.-C. Falmagne have introduced a formal notion of knowledge space to develop a scientific approach to the assessment of knowledge. They define a knowledge space as a (finite) set Q of questions together with a collection K, of subsets of Q representing different knowledge states; in addition, they assume that K is closed under set unions. The theory of knowledge spaces can be effectively connected with formal concept analysis. The connection is established by the definition of a knowledge context as a triple (P, Q, I) where P is a finite set of persons, Q is a finite set of questions, and I is a binary relation between P and Q such that plq means: the person p cannot solve the question q. For each knowledge context there is a corresponding knowledge space which consists of the complements of all intents of this context; conversely, each knowledge space can be derived in this way. Via the described connection, methods of formal concept analysis can be successfully applied to the theory of knowledge spaces. In particular, the attribute exploration as a method of conceptual knowledge acquisition yields a new approach for building knowledge spaces.
SN  - 978-3-642-80098-6
ID  - 10.1007/978-3-642-80098-6_36
ER  - 

Alternatively, I can also make a fix only in Springer Link translator, but I don't see any reason not to import this RIS field for any source if it is there. Let me know if you disagree and I am missing something.

This comment has been minimized.

Copy link
@adam3smith

adam3smith Oct 20, 2019

Collaborator

I agree with import in the absence of CI, yes -- I only checked quickly, but we do already have code that looks like it'd handle the same scenario:

if (ty == 'CONF' && entry.tags.CY && !entry.tags.C1) {

Might make sense to check if we can then take this out or alternatively use this version instead of the hard to understand degenerateImport fields? (but no strong preference either way; just don't want duplicate code)

This comment has been minimized.

Copy link
@zuphilip

zuphilip Oct 20, 2019

Author Collaborator

Okay, I will look into this more closely at a later point and also check the code you have found.

This comment has been minimized.

Copy link
@zuphilip

zuphilip Oct 21, 2019

Author Collaborator

Uff, that is a little complicated... The code you found is only considered for RIS from ProCite. When I try to take this out of this special part, then the type is not yet defined. But I cannot replace CY with C1 always, as this would destroy the cases for books as an example.

Thus, I will continue with the degenerateImport fields which I find easier to work with.

@zuphilip

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 21, 2019

Let me know @adam3smith whether you can have a look at the new version.

@adam3smith

This comment has been minimized.

Copy link
Collaborator

commented Oct 21, 2019

All good from my end. Please go ahead and squash as you see fit and merge.
Note that you refer to "BibTeX" in your last commit message when you mean RIS, so fix that if you leave the commit unsquashed.

zuphilip added 5 commits Oct 19, 2019
Moreover, add the DOI in the extra field and update
test cases. See also #1996.
- the standard map already handle CY as place for all
  cases but for conferencePapers
- conferencePaper should instead use C1 as place
- however there are cases (ProCite export, RIS from
  Springer) where also for conferencePaper CY is used
  for the place
- this is now handled directly in the degenerateMap
@zuphilip zuphilip force-pushed the zuphilip:springer-palgrave branch from 63866f5 to 73dd88b Oct 22, 2019
@zuphilip

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 22, 2019

Okay, I rebased the commits and fixed the commit messages. Thanks for the hint. Merging now.

@zuphilip zuphilip merged commit fa52663 into zotero:master Oct 22, 2019
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@zuphilip zuphilip deleted the zuphilip:springer-palgrave branch Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.