Skip to content

Quote identifier-shaped numbers (e.g. PDB code 6E72) in JSON output - #445

Open
barlowa124 wants to merge 2 commits into
project-gemmi:masterfrom
barlowa124:mmjson-numberlike-id-444
Open

barlowa124 wants to merge 2 commits into
project-gemmi:masterfrom
barlowa124:mmjson-numberlike-id-444

Conversation

@barlowa124

Copy link
Copy Markdown

Closes #444.

mmJSON (and default CIF-JSON) output wrote _entry.id 6E72 as a bare JSON value, which every JSON parser reads as the float 6e72. Real mmCIF files write such codes unquoted, so the quoting flag cannot disambiguate them.

JsonWriter::write_value now quotes tokens shaped digits + uppercase E + digits (no decimal point, no sign), next to the existing 012 exception. This matches the identifier shape of PDB codes (6E72, 7E57) while leaving real numeric tokens alone: 1.2E3, 2e4, -3E5, and 2E+6 still emit as JSON numbers.

Tested via a new case in test_cif_json.py (unfixed code produced a float, fixed produces the string) and the full suite passes on Python 3.13 (python -m unittest discover in tests/).

A token like 6E72 is a valid CIF number (6e72) but also a PDB entry ID,
and real mmCIF files write _entry.id unquoted. Emitting it bare produced
JSON that parses to a float (or Infinity in some readers), changing the
value's type. PDB codes such as 6E72 and 7E57 hit this.

A new entry in the number-vs-string check quotes values of the form
digits + E + digits with no decimal point or sign, alongside the existing
012 exception. Real numeric tokens (1.2E3, 2e4, -3E5, 2E+6) still emit
as JSON numbers.

Closes project-gemmi#444.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@CV-GPhL

CV-GPhL commented Oct 20, 2026

Copy link
Copy Markdown
Member

Thanks, this fixes 6E72 for me. I tried the patch on a few other values (mmJSON options) and have some questions about the scope of the rule.

_entry.id 6E72     -> "6E72"          (fixed)
_a 1E5             -> "1E5"           (string, was number)
_f 12E3            -> "12E3"          (string, was number)
_g 5E0             -> "5E0"           (string, was number)
_b 6e72            -> 6e72            (still a float)
_d -3E5, _e 2E+6   -> numbers         (unchanged, as described)

Questions:

  1. Genuine numbers of the shape digits + E + digits (1E5, 5E0) are now strings as well. Is that acceptable, or should the rule be narrowed to the PDB-code shape (exactly 4 characters, nonzero leading digit, e.g. 6E72, 7E57, 1E10)? Some ambiguity remains either way (12E3 is also a valid PDB code), so would a tag-aware rule (_entry.id, _database_2.database_code, data_ names) be preferable?
  2. Lowercase 6e72 is still written as a float. PDB ids do appear in lowercase in some sources, so should the check be case-insensitive?
  3. The new test uses 1.2E3 and lowercase 2e4, which do not exercise the boundary of the new exception. Could it also cover cases such as 1E5, -3E5 and 2E+6 from the PR description?

For what it's worth, extended ids (pdb_00006e72) are not affected, since the underscore already makes them strings.

Review on project-gemmi#445 showed digits-E-digits tokens of any length quoted
genuine numbers like 1E5 and 5E0. The exception now requires four
characters with a single E/e, which covers the PDB code space while
leaving other exponents numeric, and the check is case-insensitive
for lowercase ids. Boundary cases added to the JSON test.
@barlowa124

Copy link
Copy Markdown
Author

Pushed b75edd7 with the narrowed shape. The exception now requires four characters, all digits except a single E/e, so 6E72, 12E3 and lowercase 6e72 quote while 1E5, 5E0, -3E5 and 2E+6 stay numeric. The test covers those boundary cases.

A tag-aware rule would need write_value to receive the tag name, which it does not today. Threading it through the loop and map call sites adds API surface for a case that rarely occurs, since unquoted numberlike tokens in mmCIF show up in id-valued fields. The residual ambiguity (12E3 reads as both a number and a plausible code) remains without tag context either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

mmJSON converter treats entry IDs with letter "E" in them as exponential float

2 participants