frequenz-floss / setuptools-betterproto

A modern setuptools plugin to generate Python files from proto files using betterproto
https://frequenz-floss.github.io/setuptools-betterproto/
MIT License
0 stars 2 forks source link

Add Python 3.10 support #15

Open jmlemetayer opened 2 weeks ago

jmlemetayer commented 2 weeks ago

What's needed?

Ubuntu 22.04.4 LTS is using Python 3.10.12 by default.

This project seems to not have any blocking issue to support Python 3.10.

Proposed solution

Use cases

No response

Alternatives and workarounds

No response

Additional context

No response

llucax commented 2 weeks ago

PRs welcome!

jmlemetayer commented 2 weeks ago

Hum .. in fact your code is more linked to 3.11 than I thought.

import tomllib # 3.11 only
from typing import Self # 3.11 only

Also the frequenz-repo-config package is also 3.11 only (used in conftest.py). So the pytest is hard to make it work on 3.10.

I will do the work, let me know if you like it.

llucax commented 2 weeks ago

This one is easy:

-from typing import Self
+from typing_extensions import Self

For toml there is also an external package we could depend on for Python < 3.11.

llucax commented 2 weeks ago

The repo-config package is more tricky because it is a(n ugly) beast. We need to split it in smaller parts. But there is some work done to support older Python versions, coming from:

jmlemetayer commented 2 weeks ago

For toml there is also an external package we could depend on for Python < 3.11.

This is what I used but it seems to not behave the same as tomllib when mocked:

FAILED tests/test_config.py::test_from_proto_unknown_keys - TypeError: Expecting something like a string

For repo-config I have used a dirty hack with --ignore-requires-python for now ...

jmlemetayer commented 2 weeks ago

Ok, by doing this, the test passes:

-    pyproject_toml = b"""
+    pyproject_toml = """
 [tool.setuptools_betterproto]
 unknown = "unknown"
 """

But is it ok with Python 3.11 and the tomllib ?

llucax commented 2 weeks ago

It should be OK, ideally we should just use a dependency environment marker to tomllib is only installed if Python is older than 3.11, to avoid the extra unnecessary dependency, like:

dependencies = [
    "tomllib >= X.Y.Z, < X; python_version < \"3.11\""
]

Then we should try to import toml and if it fails import tomllib instead.

But I would be fine also with always installing tomllib as a first iteration towards supporting older Python versions.

jmlemetayer commented 2 weeks ago

Hum tomllib is part of Python since 3.11, and toml is an external package which seems to support 3.3+. I guess you switch both packages. But yeah that the idea!

jmlemetayer commented 2 weeks ago

PR provided. I ended up using the tomli package instead of the toml one because - like tomllib - it expects the file to be opened as a binary.

llucax commented 2 weeks ago

OK, I think we actually used tomli before Python 3.11 :+1:

So after that PR is merged these are the remaining steps to have full support for Python 3.10:

llucax commented 2 weeks ago

I will edit the issue description to add the steps towards Python 3.10 support.