-
Notifications
You must be signed in to change notification settings - Fork 36
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
update q2-sample-classifier citation #140
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.
LGTM, but, do you want to fix the title capitalization while we are at it?
Co-Authored-By: nbokulich <nbokulich@gmail.com>
thanks for committing that change @thermokarst ! LGTM. |
Did that change work for you, @nbokulich? I am still seeing a capital Q, and, when I check in q2cli it looks like those extra curly braces have been stripped. @ebolyen, does the citations API do some sanitizing when loading bibtex entries? |
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.
This is super nitpicky @nbokulich, but all of your new lines have tabs, when the rest of the file use spaces. When you have whitespace highlighting on this looks pretty gross.... :puke:
I did not test... you sounded so sure so I just assumed your fix would work! I am seeing the same thing: the extra curly braces are missing when I check in the CLI.
Nick's nits picked, see latest commit |
Re: bibtex, it is technically not using your bibtex, but reading it to generate an object for that entry, and then it re-writes it as needed. That said, the curly braces should have worked... Perhaps the Biber engine has stronger rules on title-casing? |
Yep, I could've been more clear with my question, I wanted to know what kinds of transformations were being applied to the bibtex entry. Based on my poking around, the offending config is this little bit. Looks like this is an outstanding TODO in the parser. |
@nbokulich, confirmed that the rendering of the title works fine when the parser stops stripping braces from the entries. Will merge as-is, but nothing will change on the docs side of things until the parser dependency is updated (or, we come up with another way to unicodify entries). |
@nbokulich - can you add a note to the changelog? |
No description provided.