[WIP] Fix ris import #2028
[WIP] Fix ris import #2028
Conversation
tschechlovdev
force-pushed
the
fix-ris-import
branch
2 times, most recently
from
23c261b
to
110a3f8
Compare
tschechlovdev
force-pushed
the
fix-ris-import
branch
from
110a3f8
to
112e65c
Compare
|
|
||
| String[] entries = sb.toString().replace("\u2013", "-").replace("\u2014", "--").replace("\u2015", "--") | ||
| String[] entries = linesAsString.replace("\u2013", "-").replace("\u2014", "--").replace("\u2015", "--") |
There was a problem hiding this comment.
Maybe you could extract that method to the StringUtil class, e.g. call it ReplaceUnicodeDashes..
There was a problem hiding this comment.
Done. I've also used this method in the MedlinePlain importer.
| editor = {some editor}, | ||
| language = {eng}, | ||
| number = {25}, | ||
| } |
There was a problem hiding this comment.
The name of the test files confuses me a lot...Isn't the purpose here to test the journal title and doi fields? So why are the fields not present in the file?
There was a problem hiding this comment.
Yeah you're right... Don't know how I could forget them 😄
| @@ -1,14 +1,14 @@ | |||
| @incollection{, | |||
| author = {Barata, Catarina and Celebi, MEmre and Marques, JorgeS}, | |||
| booktitle = {Dermoscopy Image Analysis}, | |||
| comment = {doi:10.1201/b19107-2}, | |||
| doi = {10.1201/b19107-2}, | |||
There was a problem hiding this comment.
I have seen that RisImporterTest3.ris specifies the both doi fields (M3 and DO) with the same doi string. Please move one of them to a different or new test file. This way the tests would have already showed that the DO field didn't import the doi.
There was a problem hiding this comment.
I've used the DO field in the TestDoiAndJournalFile.
tschechlovdev
force-pushed
the
fix-ris-import
branch
from
a71bfb9
to
18ad443
Compare
|
Travis fails, because the Architecture tests say, that the StringUtil class should be kept as small as possible ... |
|
Speak with @lenhard if it's okay to increase it and how to fix the test |
tschechlovdev
force-pushed
the
fix-ris-import
branch
from
18ad443
to
e4eb84c
Compare
|
I tried to contact @lenhard, but didn't get an answer. So for the moment I think it is the best to remove the method from the StringUtil class, so that this PR don't have to wait to long. In addition it would only be 2 classes who would use this method. Travis is still failing because of a DOI test. |
tschechlovdev
added
the
status: ready-for-review
|
Yes, it is the proper way for the moment. We can think later on for moving it. |
|
@tschechlovdev You contacted me? How? :-) This mention of myself is the first sign I see, but maybe I overlooked something. |
|
@lenhard The question was about the "Failing travis test" regarding the StringUtil method. |
|
I know I'm late to the party, but one question: is this unicode dash needed for the page numbers? Because then there is a cleanup formatter which does the job (Page number normalizer, or some permutation of these words). |
|
The reason for moving more functionality to |
* fix ris importer * fix ris import and add test cases * address comments * fix travis
* fix ris importer * fix ris import and add test cases * address comments * fix travis
Regarding: #1647
Doi import should now work with the DO field. Also I've added some fields, that are available in the RIS specification. I've also done some code cleanups.
#1074 should be also fixed now. I've added the scopus file as test file.