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

try-except tomllib import #2987

Merged
merged 5 commits into from Apr 2, 2022
Merged

try-except tomllib import #2987

merged 5 commits into from Apr 2, 2022

Conversation

JelleZijlstra
Copy link
Collaborator

@JelleZijlstra JelleZijlstra commented Apr 1, 2022

See #2983

I left the version check in place because mypy doesn't generally like try-excepted imports. I didn't try directly though.

See #2965 

I left the version check in place because mypy doesn't generally like try-excepted imports.
@pawamoy
Copy link

pawamoy commented Apr 1, 2022

Thanks 🙂 Yeah leaving the version check makes sense. We'll need to remove the marker on tomli as well in setup.py.

@ichard26
Copy link
Collaborator

ichard26 commented Apr 1, 2022

Is there a way to only select the alphas using environment markers? I'd like to avoid having 3.11 stable users also pull in tomli unnecessarily.

@JelleZijlstra
Copy link
Collaborator Author

JelleZijlstra commented Apr 1, 2022

Maybe we should just leave the setup.py markers alone and assume users testing the alphas are sophisticated enough to install tomli themselves. Or we could just remove the markers for a few months until the 3.11 betas are widely available.

CHANGES.md Outdated Show resolved Hide resolved
@pawamoy
Copy link

pawamoy commented Apr 1, 2022

Maybe we should just leave the setup.py markers alone and assume users testing the alphas are sophisticated enough to install tomli themselves.

That would defeat the purpose of this PR, which is to avoid forcing users to change their dependencies specification (adding tomli or a marker on Black) 😄

Is there a way to only select the alphas using environment markers? I'd like to avoid having 3.11 stable users also pull in tomli unnecessarily.

That's understandable. Though that would only be the case for Black versions released before 3.11 included tomllib, not the next ones. Based on PEP 508, you could use the python_full_version marker: https://peps.python.org/pep-0508/#environment-markers

@ichard26
Copy link
Collaborator

ichard26 commented Apr 1, 2022

Or we could just remove the markers for a few months until the 3.11 betas are widely available.

And yeah that's fair. I guess if any one complains about the extra dependency we could just issue a newer build number to retroactively "fix" older releases. Not perfect since the source distribution can't be updated or overridden AFAIK but oh well.

Although we should just update the markers for 3.12+, as it's 100% likely they'll have tomli.

@github-actions
Copy link

github-actions bot commented Apr 1, 2022

diff-shades reports zero changes comparing this PR (4c9c697) to main (def0483).


What is this? | Workflow run | diff-shades documentation

@JelleZijlstra JelleZijlstra requested a review from ichard26 Apr 2, 2022
Copy link
Collaborator

@ichard26 ichard26 left a comment

Can confirm 3.11.0a7 should contain tomllib. Thanks!

@JelleZijlstra JelleZijlstra merged commit 1af29fb into main Apr 2, 2022
18 checks passed
@JelleZijlstra JelleZijlstra deleted the JelleZijlstra-patch-1 branch Apr 2, 2022
@pawamoy
Copy link

pawamoy commented Apr 2, 2022

Thanks a lot for your very prompt response, PR and merge! This is very appreciated 🙂

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

3 participants