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

Fixes https://github.com/citation-style-language/abbreviations/issues/4 #6

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

retorquere
Copy link

@retorquere retorquere commented Feb 6, 2019

Fixes #4

ncbi/index.js Outdated

const abbreviations = {
"info": {
"URI": "http://www.ncbi.nlm.nih.gov/books/NBK3827/table/pubmedhelp.pubmedhelptable45/",
Copy link
Member

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.

Copy link
Author

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.

Copy link
Member

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?

Copy link
Author

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).

Copy link
Member

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)

Copy link
Author

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.

Copy link
Member

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']) {
Copy link
Member

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?

Copy link
Author

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"
Copy link
Member

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.

Copy link
Author

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",
Copy link
Member

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).

Copy link
Author

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.

Copy link
Member

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)

Copy link
Author

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.

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.

Copy link
Author

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.

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?

Copy link
Author

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.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@rmzelle
Copy link
Member

rmzelle commented Feb 7, 2019

@retorquere, thanks for this! Another question: do you also have the script for the medline abbreviations you added in the second commit?

@retorquere
Copy link
Author

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.

@rmzelle
Copy link
Member

rmzelle commented Feb 7, 2019

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)

@lewisacidic
Copy link

lewisacidic commented Feb 8, 2019

You can get the medline citationsabbreviations from the pubmed ftp servers. They might be more up to date:

ftp://ftp.ncbi.nih.gov/pubmed/J_Medline.txt

@retorquere
Copy link
Author

My build scripts get them there before building the JSON.

@lewisacidic
Copy link

Ah so they do! Sorry didn't look through the build script yet!

@retorquere
Copy link
Author

No worries.

@retorquere
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants