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

STY Uses black's with target_version >= 3.7 #20294

Merged
merged 2 commits into from Jun 17, 2021

Conversation

thomasjpfan
Copy link
Member

Follow up to #20293

This PR applies black with the target_version set.

@@ -19,6 +19,7 @@ requires = [

[tool.black]
line-length = 88
target_version = ['py37', 'py38', 'py39']
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to get that from setup.py metadata?

Copy link
Member

Choose a reason for hiding this comment

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

I think the idea is pyproject.toml to be static, to avoid all issues with dynamic behavior in setup.py. It's indeed slightly not ideal to change this for each Python version, but if you read psf/black#751 in the end as long as py37 is in there, the rest has no effect. There is no changes in formatting for Python3.7+.

@glemaitre
Copy link
Member

glemaitre commented Jun 17, 2021 via email

@thomasjpfan
Copy link
Member Author

thomasjpfan commented Jun 17, 2021

For this specific case, we can "Rebase and merge" and both commits would be on main and users can follow the workflow at: #20293 (comment)

Edit: The other PR has already been merged.

@rth
Copy link
Member

rth commented Jun 17, 2021

Can I just "Squash and merge" this one then?

@thomasjpfan thomasjpfan merged commit 351ace7 into scikit-learn:main Jun 17, 2021
@thomasjpfan
Copy link
Member Author

Can I just "Squash and merge" this one then?

Yup, I just did it. One of those rare situations where "rebase and merge" would have worked out nicer than "lets merge two PRS really quickly together".

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

Successfully merging this pull request may close these issues.

None yet

3 participants