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

Use stdlib toml provider on Python >= 3.11. Switch to tomli otherwise #20

Merged
merged 5 commits into from Sep 12, 2023

Conversation

wimglenn
Copy link
Contributor

@wimglenn wimglenn commented Sep 8, 2023

I would like to use configurator with a TOML file in Python 3.11, but lose that 3rd-party toml dependency, since Python 3.11+ has tomllib.

The current provider is not compliant with the TOML spec and does not roundtrip:

>>> import toml
>>> d = {'k': None}
>>> toml.loads(toml.dumps(d)) == d
False

For Python < 3.11, may I recommend to move to tomli, which is what stdlib tomllib was based upon. Many existing projects use tomli (e.g. pip, build, pytest, mypy, black, flit, coverage, setuptools-scm, cibuildwheel). According to PEP 680, the project uiri/toml was rejected because it's not actively maintained, does not support TOML 1.0.0 and has a number of known bugs.

Unfortunately the abstractions in configurator made it a little complicated to integrate tomli/tomllib, because the TOML specification requires the input to be UTF-8. tomli implements this requirement by forcing the user to use binary mode "b" when reading the file, then does the decoding. I've tried my best to handle TOML files in a correct way, without drastic refactoring.

Copy link
Member

@cjw296 cjw296 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally in favour of this, but not really okay with the parser-neutral methods having toml specific stuff in them...

configurator/config.py Outdated Show resolved Hide resolved
configurator/parsers.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
@wimglenn
Copy link
Contributor Author

@cjw296 It turned out to be much cleaner and simpler modifying parsers.py to use importlib. And it still works on 3.6 this way.

stream = StringIO(text)
return cls.from_stream(stream, parser)
if isinstance(text, bytes):
text = text.decode(encoding)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is this bytes?

Copy link
Contributor Author

@wimglenn wimglenn Sep 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you looking at an older commit? There should be no changes in config.py any longer.
The conversion from bytes to text is there in master currently, since 2019 acc21db

@@ -33,5 +25,5 @@ def __missing__(self, extension):
except KeyError:
raise ParseError('No parser found for {!r}'.format(extension))
else:
module = __import__(module_name)
module = import_module(module_name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@cjw296 cjw296 merged commit 8fb5f70 into simplistix:master Sep 12, 2023
6 checks passed
@cjw296
Copy link
Member

cjw296 commented Sep 12, 2023

When are you looking for a release? ASAP?

@wimglenn wimglenn deleted the stdlib-toml-loader branch September 12, 2023 17:27
@wimglenn
Copy link
Contributor Author

wimglenn commented Sep 12, 2023

Wherever it’s ready. This is for an app, so I don’t mind to lock my requirements txt directly to the git hash in the meantime.

@cjw296
Copy link
Member

cjw296 commented Sep 13, 2023

https://pypi.org/project/configurator/3.2.0/ for ya!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants