-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Fixes https://github.com/citation-style-language/abbreviations/issues/4 #6
base: master
Are you sure you want to change the base?
Conversation
ncbi/index.js
Outdated
|
||
const abbreviations = { | ||
"info": { | ||
"URI": "http://www.ncbi.nlm.nih.gov/books/NBK3827/table/pubmedhelp.pubmedhelptable45/", |
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.
Is this URI correct? When I visit it I get a "Page not available" error.
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.
It's not correct, but I tried to change as little as possible from the previous python script. I've changed it now to the correct URL.
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.
It's not correct, but I tried to change as little as possible from the previous python script.
Ah, I didn't realize you had updated @cpina's original code. Carles, does Mendeley depend on this repo at all?
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.
It'd be pretty easy to revert, I missed that this isn't a Zotero repo, and I was looking to simplify things in terms of formats (Zotero uses JSON for abbrev lists) and tools (Zotero is JS-based).
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.
It's probably fine as long as the abbreviation lists can also be accessed directly through the repository. (the original discussion that led to this repo is at https://sourceforge.net/p/xbiblio/mailman/xbiblio-devel/thread/CAHUwU6uovCXizOzUnM%2BVBufbX7YonraY0K7o-fnpqwVGRJ06RA%40mail.gmail.com/#msg31060169)
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.
In what format though? JSON? tab-separated text? Both? Easy enough to do.
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.
It's not correct, but I tried to change as little as possible from the previous python script.
Ah, I didn't realize you had updated @cpina's original code. Carles, does Mendeley depend on this repo at all?
No, Mendeley doesn't depen on this repo so it can be changed, no problems.
}, | ||
} | ||
|
||
for (const list of ['J_Entrez.txt', 'J_Medline.txt', 'J_Sequence.txt']) { |
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.
Where did you get these files (J_Entrez.txt', 'J_Medline.txt', 'J_Sequence.txt
) from?
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.
They were already checked into this repository. The updated script will fetch them during build -- we could probably remove them from the repo and add them to .gitignore
package.json
Outdated
}, | ||
"repository": { | ||
"type": "git", | ||
"url": "git+https://github.com/retorquere/abbreviations.git" |
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.
Should we change this to citation-style-language org? Same for the other URLs in this file.
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.
Done.
package.json
Outdated
@@ -0,0 +1,23 @@ | |||
{ | |||
"name": "zotero-abbreviations", |
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.
Can we brand this as CSL instead of Zotero? There is nothing here that is Zotero-specific, right?
And do I understand correctly that this package.json is for publishing on npm? Somebody just managed to get hold off the @csl
npm organization, so we could publish under that. See citation-js/citation-js#6 (comment).
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.
Done -- I did plan to get these to npmjs (which I think is a good way to distribute these -- I need them myself and a good distribution platform would help me) but I held off on actually publishing it on npmjs because I did not want to claim ownership.
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.
I've never really worked with npm, so maybe you and @richlewis42 could work together to get the npm package published? As we discussed in the linked issue above, it would also be good to leave a note at https://discourse.citationstyles.org/ so that people are aware we're planning to start using npm, in case other folks have suggestions on how to best organize the different packages.
(somebody close to CSL, like @adam3smith and/or myself, should also get the credentials for the npm @csl
account)
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.
I'd be happy to help getting it done. npm publishing is really easy, not much to 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.
@rmzelle More than happy to transfer the @csl
organisation credentials - do you have an npm account? I'm sorry I haven't made that post yet, I've got a lot of things going on so put this on the backburner.
@retorquere I've added you to the org, so you should be able to publish the repo there when its ready.
With regards to the package itself, I would suggest that the index.js
exports the JSON (or just inline the objects), that is what I'm doing for the styles and the locales (wip). That way you could just
import abbreviations from '@csl/abbreviations'
Just like you will be able to
import style from '@csl/style-vancouver'
import locale from '@csl/locale-en-us'
(or
import style from '@csl/styles/vancouver'
import locale from '@csl/locales/en-us'
depending on whether we do a big styles repo or lots of small ones)
This stops you worrying about json webpack loaders or anything 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.
The abbreviation lists can get quite big (and I have at least 4 more lined up); having the index.js
export the abbreviations would mean you'd always load every abbreviation list in memory.
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.
Yeah I suppose so (+add a bunch of weight to any bundle if its used in browser). Still, wouldn't it be more useful than not doing it?
And if its in JSON aren't you likely to have to load it all in anyway if you are going to use 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.
I wouldn't. I'm working on a plugin (natch) that would allow switching lists in current Zotero, and I'd stash load them separately on demand. Having the index.js
export a list of names would be useful, but then require('@csl/abbreviations')
would just be equivalent to require('@csl/abbreviations/package.json').files
.
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.
I'm just suggesting it to provide a simple API for the package. It should probably export something if it's on NPM. Not a big issue right now though.
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.
Done.
@retorquere, thanks for this! Another question: do you also have the script for the medline abbreviations you added in the second commit? |
The medline abbreviations I just copied from https://github.com/zotero/zotero/blob/master/resource/schema/abbreviations.json -- I don't know how that was put together. |
Yeah, I think we never were able to get those details from the Zotero devs. (cc @adam3smith) |
You can get the medline ftp://ftp.ncbi.nih.gov/pubmed/J_Medline.txt |
My build scripts get them there before building the JSON. |
Ah so they do! Sorry didn't look through the build script yet! |
No worries. |
Note though that the abbreviations I load from ftp://ftp.ncbi.nih.gov/pubmed are not the same as the medline abbrev list embedded in Zotero, which also has a abbreviation word list in addition to a simple journal list. |
Fixes #4