Skip to content
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

Forthcoming changes in TOML v1.1.0 will cause TOMLDecodeError. tomllib.loads doesn't support optional seconds in times. #103992

Closed
JamesParrott opened this issue Apr 29, 2023 · 5 comments
Labels
3.11 only security fixes 3.12 bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@JamesParrott
Copy link

JamesParrott commented Apr 29, 2023

[Edit] I erroneously assumed the "minimal configuration file format", that I personally find to be a pleasant alternative to JSON, was done and dusted. But TOML is still very much under active development. On its main branch.

Python 3.12 is compliant with TOML v1.0.0 as far as I'm aware. But optional seconds, and other issues will arise if TOML v1.1.0 files are attempted to be loaded with tomllib. This is absolutely not a request to support TOML v1.1.0.

This issue is an informational advanced warning only, of changes coming down the line. Feel free to decide not to support them all (TOML v1.1 is completely unnecessary for my projects, and I won't be supporting it). But if so, it's still good to make a decision, and record that somewhere. If nothing else so it is clear what should be included in test data.

Background: In summer last year (2022) the TOML language's latest version relaxed the requirement for seconds to be specified in a time: toml-lang/toml@7e8a748

A test for optional seconds was also added to toml-test's development version: https://github.com/BurntSushi/toml-test/blob/master/tests/valid/datetime/no-seconds.toml

Implications: In Python 3.12, tomllib requires seconds to be present. Currently in the absence of seconds, the regex _TIME_RE_STR does not match. Otherwise both local variables called sec_str could not be defined. So if others decide to support TOML v1.1, this will affects all platforms.

Regex:

_TIME_RE_STR = r"([01][0-9]|2[0-3]):([0-5][0-9]):([0-5][0-9])(?:\.([0-9]{1,6})[0-9]*)?"

sec_str:
hour, minute, sec = int(hour_str), int(minute_str), int(sec_str)

sec_str:
return time(int(hour_str), int(minute_str), int(sec_str), micros)

Bug report

Python 3.12.0a7 (tags/v3.12.0a7:b861ba4, Apr  4 2023, 16:33:41) [MSC v.1934 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import tomllib
>>> tomllib.loads('t=12:00:00\n')
{'t': datetime.time(12, 0)}
>>> tomllib.loads('t=12:00\n')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Program Files\Python312\Lib\tomllib\_parser.py", line 127, in loads
    raise suffixed_err(
tomllib.TOMLDecodeError: Expected newline or end of document after a statement (at line 1, column 5)
>>>
@JamesParrott JamesParrott added the type-bug An unexpected behavior, bug, or error label Apr 29, 2023
@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 29, 2023

Cc. @hauntsaninja / @hukkin for tomllib

@JamesParrott JamesParrott changed the title TOMLDecodeError doesn't support optional seconds in times. TOML does since July 2022. Still v1.0.0 apparently TOMLDecodeError. tomllib.loads doesn't support optional seconds in times. TOML does since July 2022. Still v1.0.0 apparently Apr 29, 2023
@JamesParrott
Copy link
Author

It's mentioned on the TOML github: https://github.com/toml-lang/toml/blob/main/toml.md#user-content-local-time
but not currently on the spec on the toml website: https://toml.io/en/v1.0.0#local-time

It looks to me like the decision to allow optional seconds was taken, but could have been better communicated.

@AlexWaygood AlexWaygood added stdlib Python modules in the Lib dir 3.11 only security fixes 3.12 bugs and security fixes labels Apr 29, 2023
@JamesParrott JamesParrott changed the title TOMLDecodeError. tomllib.loads doesn't support optional seconds in times. TOML does since July 2022. Still v1.0.0 apparently Forthcoming changes in TOML v1.1.0 will cause TOMLDecodeError. tomllib.loads doesn't support optional seconds in times. Apr 29, 2023
@hauntsaninja
Copy link
Contributor

hauntsaninja commented Apr 29, 2023

Thanks for bringing this up!

hukkin has done an excellent job staying on top of changes and has a PR for this specific change ready to go: https://github.com/hukkin/tomli/pull/203/files

PEP 680 specifies we can treat minor revisions to TOML as bug fixes, so when v1.1 is released we should have the option to backport it if we think that's wise: https://peps.python.org/pep-0680/#stability-of-toml Currently defaulting to avoid shipping changes until v1.1 is released: toml-lang/toml#928

cc @pradyunsg in case you have opinions for how the release cycle of TOML and CPython should intersect :-)

@JamesParrott
Copy link
Author

You're welcome. Great job hukkin!

I was just very surprised earlier today to see a Python core library 'fail' a toml-test. But I'd got my wires crossed, and didn't realise I was using cutting edge tests, for features ahead of Toml 1.0.0

@pradyunsg
Copy link
Member

pradyunsg commented Apr 29, 2023

cc @pradyunsg in case you have opinions for how the release cycle of TOML and CPython should intersect :-)

We should be able to pull it in the next minor version of Python, once "upstream" (AKA me) cuts TOML v1.1.0.

I don't think we should try backporting to an older minor version in a patch release, but anything stated in the PEP takes precedence over my opinions. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants