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.

Sign up for GitHub

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

Jump to bottom

RFC: from __future__ import annotations + pyupgrade #10997

Closed
wants to merge 2 commits into from
Closed

RFC: from __future__ import annotations + pyupgrade #10997

wants to merge 2 commits into from

Conversation

RonnyPfannschmidt
Copy link
Member

No description provided.

@RonnyPfannschmidt RonnyPfannschmidt marked this pull request as draft May 13, 2023 11:59
@RonnyPfannschmidt RonnyPfannschmidt requested review from bluetech, nicoddemus and asottile and removed request for nicoddemus May 13, 2023 11:59
asottile
asottile approved these changes May 13, 2023
View reviewed changes
Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

noice

nicoddemus
nicoddemus approved these changes May 13, 2023
View reviewed changes
doc/en/conf.py Show resolved Hide resolved
@RonnyPfannschmidt RonnyPfannschmidt marked this pull request as ready for review May 13, 2023 16:51
bluetech
bluetech approved these changes May 19, 2023
View reviewed changes
Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Would be nice to be able to use the new syntax!

@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus this one has a major total anhillating force for the automatic backporting

Any idea how we can work with that (or do we backport this one's)

Once we have a idea on backport I'll fix it up and see it through

@Pierre-Sassoulas
Copy link
Member

Any idea how we can work with that (or do we backport this one's)

Yeah, it needs either a backport itself or to wait for the release of the next minor being imminent before merging. Depending on the confidence we have it's not going to break the maintenance branch..

@nicoddemus
Copy link
Member

nicoddemus commented May 19, 2023 •
edited
Loading

Depending on the confidence we have it's not going to break the maintenance branch..

That's the main point I think. Can we think of cases where backporting this could break downstream users somehow in a patch release?

@RonnyPfannschmidt
Copy link
Member Author

Let's give it a call out, we might Learn of really interesting (or horrifying) use cases for the annotations

Im going to take a look at fixup /backport fixup soonish

@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus @bluetech now that 8x. and 7.4 had some major releases, i'd like to propose porting main and 8.x over to the new syntax, i'll try to get it tried this weekend

@RonnyPfannschmidt RonnyPfannschmidt force-pushed the ronny/typing-annotations branch 2 times, most recently from 394560a to c172383 Compare January 5, 2024 11:09
@RonnyPfannschmidt
Copy link
Member Author

this pr as is may be a complete pain to review - i wonder if i ought to split it across some logical/file boundaries

bluetech
bluetech approved these changes Jan 5, 2024
View reviewed changes
Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

A few formatting comments but LGTM. I suggest merging quickly to avoid conflicts.

src/_pytest/_code/code.py
Comment on lines +717 to +718
expected_exception: builtins.type[BaseException]
| tuple[builtins.type[BaseException], ...],
Copy link
Member

Choose a reason for hiding this comment

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

Would wrap in parens to make the formatting less weird

src/_pytest/_code/code.py
Comment on lines +747 to +748
expected_exception: builtins.type[BaseException]
| tuple[builtins.type[BaseException], ...],
Copy link
Member

Choose a reason for hiding this comment

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

Would wrap in parens to make the formatting less weird

src/_pytest/fixtures.py
ids: Optional[
Union[Tuple[Optional[object], ...], Callable[[Any], Optional[object]]]
] = None,
ids: None | (tuple[object | None, ...] | Callable[[Any], object | None]) = None,
Copy link
Member

Choose a reason for hiding this comment

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

Redundant parens

src/_pytest/fixtures.py
Union[Tuple[Optional[object], ...], Callable[[Any], Optional[object]]]
] = None
name: Optional[str] = None
ids: None | (tuple[object | None, ...] | Callable[[Any], object | None]) = None
Copy link
Member

Choose a reason for hiding this comment

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

Redundant parens

src/_pytest/fixtures.py
Union[Sequence[Optional[object]], Callable[[Any], Optional[object]]]
] = ...,
name: Optional[str] = ...,
ids: None | (Sequence[object | None] | Callable[[Any], object | None]) = ...,
Copy link
Member

Choose a reason for hiding this comment

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

Redundant parens

src/_pytest/fixtures.py
Union[Sequence[Optional[object]], Callable[[Any], Optional[object]]]
] = ...,
name: Optional[str] = None,
ids: None | (Sequence[object | None] | Callable[[Any], object | None]) = ...,
Copy link
Member

Choose a reason for hiding this comment

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

Redundant parens

src/_pytest/fixtures.py
] = None,
name: Optional[str] = None,
) -> Union[FixtureFunctionMarker, FixtureFunction]:
ids: None | (Sequence[object | None] | Callable[[Any], object | None]) = None,
Copy link
Member

Choose a reason for hiding this comment

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

Redundant parens

src/_pytest/mark/structures.py
Comment on lines +471 to +475
ids: None
| (
Iterable[None | str | float | int | bool]
| Callable[[Any], object | None]
) = ...,
Copy link
Member

Choose a reason for hiding this comment

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

Redundant parens & weird formatting

src/_pytest/python.py
argnames: str | Sequence[str],
argvalues: Iterable[ParameterSet | Sequence[object] | object],
indirect: bool | Sequence[str] = False,
ids: None | (Iterable[object | None] | Callable[[Any], object | None]) = None,
Copy link
Member

Choose a reason for hiding this comment

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

Redundant parens

@nicoddemus
Copy link
Member

I suggest merging quickly to avoid conflicts.

Agree, I won't have time to review this in a timely manner, but please go ahead. Also I suggest to squash the commits into a single one, as the last two are just fixups.

@RonnyPfannschmidt
Copy link
Member Author

i'll add a few more fixups, then squash and submit for final merge

@RonnyPfannschmidt RonnyPfannschmidt mentioned this pull request Feb 21, 2024
Open
4 tasks
@RonnyPfannschmidt RonnyPfannschmidt force-pushed the ronny/typing-annotations branch from c172383 to c6175e0 Compare June 17, 2024 12:56
@RonnyPfannschmidt
Copy link
Member Author

closing in favor of #12467

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicoddemus nicoddemus nicoddemus approved these changes

@asottile asottile asottile approved these changes

@bluetech bluetech bluetech approved these changes

Assignees

@RonnyPfannschmidt RonnyPfannschmidt

Labels
None yet
Projects
pytest sprint 2024
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@RonnyPfannschmidt @Pierre-Sassoulas @nicoddemus @bluetech @asottile