Skip to content

model: parse 1-7 digit fractional seconds in datetime literals on Python 3.10 - #323

Closed
VitorCoelho wants to merge 3 commits into
SAP:masterfrom
VitorCoelho:fix/datetime-fractional-seconds
Closed

VitorCoelho wants to merge 3 commits into
SAP:masterfrom
VitorCoelho:fix/datetime-fractional-seconds

Conversation

@VitorCoelho

Copy link
Copy Markdown

What

Edm.DateTime and Edm.DateTimeOffset literal parsing now normalizes the fractional seconds to exactly 6 digits (pad or truncate) through one small helper, normalize_fractional_seconds, before calling datetime.fromisoformat.

Why

#303 switched to fromisoformat and padded the fraction, because Python 3.10 (still supported and in the CI matrix) accepts only 3 or 6 digits. Two cases were still broken on 3.10:

  • Edm.DateTimeOffset took the first 6 characters after the dot as the fraction, so a shorter fraction swallowed part of the offset: datetimeoffset'1976-11-23T03:33:06.5+01:00' and datetimeoffset'...06.12Z' raised PyODataModelError.
  • 7-digit fractions (fffffff, allowed by OData V2 and mentioned in the Edm.DateTime docstring) were padded but never truncated, so both types failed.

Python 3.11+ accepts these natively and truncates to microseconds; the helper gives 3.10 the same result.

How it was tested

  • New cases in tests/test_model_v2.py: 7-digit Edm.DateTime; Edm.DateTimeOffset with 1, 2 and 7 digits plus offset/Z.
  • Without the fix, on Python 3.10: 4 of the new cases fail. With the fix: full suite passes on Python 3.10 and 3.13 (301 passed).
  • CHANGELOG entry added under Unreleased / Fixed.

…hon 3.10

On Python 3.10, datetime.fromisoformat accepts only 3 or 6 digit
fractions. The padding added in #303 did not cover two cases:

- Edm.DateTimeOffset took the first 6 characters after the dot as the
  fraction, so a shorter fraction swallowed part of the offset and
  datetimeoffset'...06.5+01:00' failed to parse.
- Seven digit fractions (fffffff, allowed by OData V2) were padded but
  never truncated, so both Edm.DateTime and Edm.DateTimeOffset failed.

Normalize the fraction to exactly 6 digits with one shared helper,
matching what Python 3.11+ does natively (truncation).

Signed-off-by: VitorCoelho <vitorc195@gmail.com>
The new cases fail on Python 3.10 without the previous commit.

Signed-off-by: VitorCoelho <vitorc195@gmail.com>
Signed-off-by: VitorCoelho <vitorc195@gmail.com>
@phanak-sap

phanak-sap commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Hi @VitorCoelho,
thx for suggestion, this feels like temporary helper refactoring for 3.10 python only, than necessary bugfix. While the code is correct, I would close this PR anyway from following reasons:

  • while we are intentionally bit lagging behind Python versions in support, in pyodata 1.13 or definitely in 1.14 (if some feature would enforce creating own MINOR version) the minimum Python version will be 3.11. It is already quite after EOL and even 3.11 is quite long out.
  • with that, all things mentioned in fromisotime cleanup when 3.11 is Python miniumum #304 will be done as well and no helper should be IMHO needed anymore. "remove fractional-seconds padding workaround" is second item there.
  • the DateTime split is the one of most "in the face" change between V2 and V4 odata. I would use the "3.11 is minimal + all refactoring cleaned" as a basis for starting of V4 support. At the moment it lives in experimental branch, refer to https://github.com/SAP/python-pyodata/wiki/Road-to-OData-V4 for detail. Since you are so eager to help, I will enjoy any testing effort (at the moment I tried that code against Nothwind sample service - Batch and FunctionImports/Batches are not working at all, but hey, at least it parses metadata and most of the /docs examples seems to work)

@phanak-sap
phanak-sap marked this pull request as draft October 25, 2026 18:11
@VitorCoelho

VitorCoelho commented Oct 25, 2026 •

Copy link
Copy Markdown
Author

Thanks for the context. That makes sense: with 3.11 as the minimum the workaround goes away anyway, so there's no point patching it now.

@phanak-sap

Copy link
Copy Markdown
Contributor

Thanks for the context. That makes sense: with 3.11 as the minimum the workaround goes away anyway, so there's no point patching it now.

OK, closing on mutual agreement.

@phanak-sap phanak-sap closed this Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants