-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add DOIs to proceedings and collections #70
Conversation
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.
A quick comment bc I'd like to approve this PR as i completely agree with its conceptual goal.
Shouldn't the line read:
"format.pid output"
?
Yes. Good grief. I'm sorry. Now I'll update with the actual change that works... |
SPIE papers come with DOIs and ADS includes them in entries. It seems odd to me that we include DOIs for articles but not for proceedings papers that have them. SPIE papers are notoriously hard to parse since page numbers are usually missing from references.
17c506f
to
624c2f8
Compare
Updated. Sorry about that. Trying not to commit the removal of trailing whitespace confused me a bit. |
Doing some testing. Goodness the ADS BibTeX is not that much better than the SPIE bibtex. I hope to approve this soon. |
As a note the test case is: https://doi.org/10.1117/12.856347 or https://ui.adsabs.harvard.edu/#abs/2010SPIE.7735E..0NB/abstract |
The SPIE version is awful since it uses commas to separate the authors and the pages entry is actually incorrect. ADS is better since for pages it properly uses the eid. Neither include the proceedings editors (ADS do try to fix that but the problem is upstream with SPIE). |
I added the DOI to |
The ADS bibtex is much better wrt the authors but there is currently a mess (in my opinion; see the duplicated Volume/eid in the compiled version below) where ADS includes the Volume/eid are included in booktitle. As there really isn't a work around in a bst for variable bibtex, this doesn't appear related to the PR. I want to check out a few more use cases if that is okay @timj
|
That's odd. For example, I'm looking at http://adsabs.harvard.edu/abs/2016SPIE.9911E..18A:
and in my PDF I get:
Which seems okay to me. I don't get any |
Right, sometimes ADS is including the Volume/eid/page into the booktitle as in these two (really quite random) examples:
|
Wow. I've just looked and it seems to be that you are running into a new bumble bee bug. If I look at the same bibtex entry on bumble bee at https://ui.adsabs.harvard.edu/#abs/2016SPIE.9911E..18A/abstract I get:
which is very wrong. @aaccomazzi any thoughts? Bumble bee was working fine a month back in February (I thought). |
And your ADASS example is fine on classic:
|
Oh, and bumblebee is not even including the |
Looks like this is a problem with the generation of metadata fields for conference proceedings in our new system. I've opened an issue about it (see above). @timj the reason why these entries were "right" in bumblebee earlier was because we used to simply serve output from ADS classic, whereas now we natively create these entries from our new microservices. The usual bumps on the road... |
good times. |
SPIE papers come with DOIs and ADS includes them in entries. It seems odd to me that we include DOIs for articles but not for proceedings papers that have them. SPIE papers are notoriously hard to parse since page numbers are usually missing from references.