Fix compatibility with python 3.8 and 3.9 #245
Conversation
…of '|' in favor of Union
|
@raphaeljolivet Thanks! Can you give us an indication of why 3.8 compatibility is needed? It is EOL in 2024, and even if we add compatibility, it would limit us to dependencies which are also 3.8 compatible. I don't remember the details but some of the CI workflows also broke for 3.8 (though maybe this was for another component library). Normally we don't need |
|
The compatilibity to Python 3.8 could be optional indeed. I am in the process of releasing a new version of lca_algebraic. I have updated to Brightway 2.45 (maybe later to brightway25) : And I have setup sone tests on tox. It installed without error on python3.8 and python3.9 but failed at runtime. To me a library should not install if it fails at runtime : For 3.8, while looking at it, I found that it only required a couple of change to install and run smoothly, and I thought it was a pitty to not support it : But indeed I don't have the full picture. If maintaining 3.8 is a burden for the dependencies, you could drop its support and advertise the min python version in the manifest of the library. |
|
And thanks for your work btw :) |
|
@raphaeljolivet right back at you :) |
bw2io/ecoinvent.py
Outdated
|
|
||
| import bw2data as bd | ||
| import ecoinvent_interface as ei | ||
| from charset_normalizer.md import List |
bw2io/ecoinvent.py
Outdated
| @@ -62,11 +63,11 @@ def pick_a_unit_label_already(obj: dict) -> str: | |||
| def import_ecoinvent_release( | |||
| version: str, | |||
| system_model: str, | |||
| username: str | None = None, | |||
| password: str | None = None, | |||
| username: Union[str, None] = None, | |||
There was a problem hiding this comment.
Please change this, and future instances of this pattern, to use Optional instead.
|
While you're at it, can you update |
This is a fix for #244
It's basically changing :
type | type2toUnion[type, type2]Please consider using Tox to ensure broad compatibility with several version of Python.