Fix lists - Migrate from semicolon to comma as separator#139
Conversation
|
Hi, thanks for your work, as issue #130 , I have completed csv migration from semicolon to comma with csv file and all scripts, please review the code. Thanks! |
koppor
left a comment
There was a problem hiding this comment.
Oh, wow, looks very awesome! Thank you for taking your time.
I have a small "nitpick" regarding the readability of the CSV files. Now all items are quoted. I find that more hard to read than before. What's your take on that?
| Zhipu Xuebao;Zhipu Xuebao | ||
| Zhongguo Shengwu Huaxue Yu Fenzi Shengwu Xuebao;Zhongguo Shengwu Huaxue Yu Fenzi Shengwu Xuebao | ||
| Zhongguo Wuji Fenxi Huaxue;Zhongguo Wuji Fenxi Huaxue | ||
| "ACS Applied Materials & Interfaces","ACS Appl. Mater. Interfaces" |
There was a problem hiding this comment.
Do we really need " everywhere? Wouldn't it be possible to use enclsoing quotes only when needed?
There was a problem hiding this comment.
Honestly, it's my preference, I'm used to making all the formatting consistent, and when there's a value that needs to have quotes added to it, I always want to make the other values consistent with that value. Not only in CSV documents, but also when I format js objects.
I don't think all quotes affect the readability of the data, it clearly tells the user that what is between each pair of quotes is continuous text.
If the JabRef team thinks that "use quotes only when necessary" is a better solution, I can change it over.
There was a problem hiding this comment.
We discussed internally and follow your flow!
| data = list(csv_reader) | ||
|
|
||
| with open(output_file, 'w', newline='', encoding='utf-8') as outfile: | ||
| csv_writer = csv.writer(outfile, delimiter=',', quoting=csv.QUOTE_ALL) |
There was a problem hiding this comment.
I think, QUOTE_ALL should be replaced by something else
| # End of https://www.toptal.com/developers/gitignore/api/linux,windows,intellij+all,microsoftoffice,latex | ||
|
|
||
| # For repo | ||
| /*.csv No newline at end of file |
There was a problem hiding this comment.
Why this? I think, this has to be removed - we do version csv files.
There was a problem hiding this comment.
This ignore only affects csv files in the root directory of the repository, which are usually generated by script combining or processing, and in fact are not checked into the repository. journals/*.csv is not affected.
|
Needed to update the update scripts also to use |
| Biblioteca di Bibliografia Italiana;Bibl. Bibliogr. Ital. | ||
| Biblioteca de la Ciencia Española;Bibl. Cienc. Esp. | ||
| Bibliothèque d'Histoire des Sciences;Bibl. Hist. Sci. | ||
| \cyr Biblioteka Matematika;Bibl. Mat. |
There was a problem hiding this comment.
Not sure what these escapings (\cyr) etc. mean
Fixes #130