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

Remove the f-string self-documenting expression for better Python 3.7 support #78

Merged
merged 1 commit into from
Jan 16, 2024
Merged

Conversation

adokitkat
Copy link
Contributor

Hello.

I know Python 3.7 isn't officially supported, but it seems to work with this small change. It would be great if this change gets accepted because then it would make it easier to integrate with esp-idf versions v5.0 and v5.1, because their minimal Python version is indeed 3.7 (only the latest esp-idf v5.2 bumps this to 3.8).

The current f-string = self-documenting expression breaks our internal CI when trying to backport official littlefs example from v5.2 to v5.1 and v5.0 because they use the older Python version.

Thank you!

@adokitkat adokitkat changed the title Remove f-string self-documenting expression for a better Python 3.7 support Remove the f-string self-documenting expression for better Python 3.7 support Jan 15, 2024
@BrianPugh
Copy link
Collaborator

Isn't it possible to update the version of python within esp-idf? While it's noble (and in this case, fairly trivial) to try and support older versions of python, python3.7 hit its EOL 6 months ago.

Considering esp-idf v5.1 EOL is Dec 2025, I think it's unreasonable to support a version of python 2.5 years past its EOL.

Basically, this would set the precedent that all updates/PRs have to be python3.7 compatible for the next 1.5 years, which I'm not a fan of.

@adokitkat
Copy link
Contributor Author

I agree it would be a better solution, however I don't think I'll be able to do that (can find out more info tomorrow, but likely not). Certain tagged versions have certain minimal Python version requirements.

However I am not saying it needs to be officially compatible with future versions of this package as long as we have at least one working - the example in older v5.0 and v5.1 branches will just use the last working version so the CI which uses minimal version will work, while this version number can be changed by a user anyway to a higher version as long they use newer version of Python on their system which is likely (and we can also add a recommendation to readme file to use Python 3.8+ for full compatibility). I hope you understand what I mean.

I will get back to you tomorrow about rising the IDF minimal Python requirement.

@BrianPugh
Copy link
Collaborator

Let me know if you guys are able to use a more modern python version. If not, I'm OK doing a 1-time python3.7 release. Would you guys be fine with just a source distribution? Otherwise we'll have to configure (and revert) CI to create prebuilt wheels for python3.7.

@adokitkat
Copy link
Contributor Author

Hi again. So unfortunately I am not able to raise the minimal Python version for older IDF releases. So we'll have to proceed with this MR. Source distribution released on PyPI should be enough. Thank you very much!

@BrianPugh BrianPugh merged commit bbf7362 into jrast:master Jan 16, 2024
12 checks passed
@BrianPugh
Copy link
Collaborator

v0.9.3 has been released, which should have a python3.7-compatible source distribution.

BrianPugh pushed a commit to joltwallet/esp_littlefs that referenced this pull request Mar 6, 2024
Due to ESP-IDF v5.0 and v5.1 supporting Python 3.7
Related jrast/littlefs-python#78
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.

2 participants