-
Notifications
You must be signed in to change notification settings - Fork 5
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
ciftools is not allowing flexibility in case for attributes #3
Comments
Good point, @BobHanson. The implementation indeed diverges from the format specification. However, it provides the same behavior as the original JS impl by David. I'll have a look at it and potentially will disentangle that functionality from #2. |
OK, great. Sorry about the convoluted push. I was not expecting it to join
all the commits into one pull request, but I guess that makes sense. As
long as we all agree that case-insensitivity is important, I'm sure you can
handle it better than I did. One option I didn't try but would of course be
a very simple one would be to use .toLowerCase() in the MessagePack map
binary reading (probably not map writing). I chose to do it later because I
thought perhaps a read/write cycle should preserve the case of the
attributes that were read. Totally above my pay grade to think about that.
…On Mon, Dec 2, 2019 at 5:26 PM Sebastian Bittrich ***@***.***> wrote:
Good point, @BobHanson <https://github.com/BobHanson>. The implementation
indeed diverges from the format specification. However, it provides the
same behavior as the original JS impl
<https://github.com/dsehnal/CIFTools.js/blob/34cd913b12af9a0ebb9baca99af79903ab8989d7/src/Text/Parser.ts#L14>
by David. I'll have a look at it and potentially will disentangle that
functionality from #2 <#2>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3?email_source=notifications&email_token=AEHNCW3LW5CDNQEESUXLDA3QWWYT7A5CNFSM4JTBSNOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFXYDKY#issuecomment-560955819>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEHNCW34UPW5MYOZGMK4H3TQWWYT7ANCNFSM4JTBSNOA>
.
--
Robert M. Hanson
Professor of Chemistry
St. Olaf College
Northfield, MN
http://www.stolaf.edu/people/hansonr
If nature does not answer first what we want,
it is better to take what answer we get.
-- Josiah Willard Gibbs, Lecture XXX, Monday, February 5, 1900
|
Did some reading. A really elegant solution would be to use Handling of category/column names is case insensitive as of a2f798e. |
Great. I would go for the simplest solution possible. (please. ;)
…On Tue, Dec 3, 2019 at 11:44 AM Sebastian Bittrich ***@***.***> wrote:
Did some reading. A really elegant solution would be to use new
TreeMap<String, Object>(String.CASE_INSENSITIVE_ORDER) which keeps the
keys case sensitive while making get/put/et al case-insensitive. However,
that enforces the natural ordering of all map entries.
Handling of category/column names is case insensitive as of a2f798e
<a2f798e>.
CaseSensitivityTest defines the behavior for reading and writing. Thanks
again for the input.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3?email_source=notifications&email_token=AEHNCW7SJAKMTN2K44PDYSLQW2ZJZA5CNFSM4JTBSNOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEF2SQLY#issuecomment-561326127>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEHNCW53MOYYP7VMRINBLU3QW2ZJZANCNFSM4JTBSNOA>
.
--
Robert M. Hanson
Professor of Chemistry
St. Olaf College
Northfield, MN
http://www.stolaf.edu/people/hansonr
If nature does not answer first what we want,
it is better to take what answer we get.
-- Josiah Willard Gibbs, Lecture XXX, Monday, February 5, 1900
|
ciftools-java is requiring exact-case matches to its category and column names. However, http://mmcif.wwpdb.org/docs/tutorials/mechanics/pdbx-mmcif-syntax.html:
Data category and data item names are not case sensitive.
corresponding to
https://www.iucr.org/resources/cif/spec/version1.1/cifsyntax#case
_Case sensitivity
Pull #2 addresses this issue; I thought it would be a separate pull, but I see now that it just attached one push to another. So sorry for that. Included there:
The text was updated successfully, but these errors were encountered: