Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Consolidate descriptor handling in checkmember.py#18831

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ilevkivskyi merged 4 commits intopython:masterfromilevkivskyi:move-descriptor-set
Mar 24, 2025

Conversation

@ilevkivskyi
Copy link
Member

@ilevkivskyiilevkivskyi commentedMar 22, 2025•
edited
Loading

This is not a pure refactoring, but almost. Right now we are in a weird situation where we have two inconsistencies:

  • __set__() is handled inchecker.py while__get__() is handled incheckmember.py
  • rules for when to use binder are slightly different between descriptors and settable properties.

This PR fixes these two things. As a nice bonus we should get free support for unions in__set__().

@github-actions

This comment has been minimized.

@ilevkivskyi
Copy link
MemberAuthor

Hm, couple new errors indd-trace-py raises an interesting question, whether we need to invoke descriptors when they appear as aprotocol attribute? Although they are technically defined in the class body, IIUC the intention for them is to be instance variables. Normally we skip descriptors if the variable isimplicit (i.e. was set onself), but there is no such thing in protocols.

I will try this later today, just to check the possible fallout.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

Diff frommypy_primer, showing the effect of this PR on open source code:

kornia (https://github.com/kornia/kornia)- kornia/augmentation/container/augment.py:449: error: Unsupported right operand type for in ("list[DataKey] | None")  [operator]- kornia/augmentation/container/augment.py:450: error: Item "None" of "list[DataKey] | None" has no attribute "index"  [union-attr]+ kornia/augmentation/container/augment.py:321: error: Unused "type: ignore" comment  [unused-ignore]+ kornia/augmentation/container/augment.py:323: error: Unused "type: ignore" comment  [unused-ignore]+ kornia/augmentation/container/augment.py:443: error: Unused "type: ignore" comment  [unused-ignore]+ kornia/augmentation/container/augment.py:445: error: Unused "type: ignore" comment  [unused-ignore]steam.py (https://github.com/Gobot1234/steam.py)+ steam/state.py:2427: error: Argument "targetid" to "AddReactionRequest" has incompatible type "int | _ReadOnlyProto[int]"; expected "int"  [arg-type]+ steam/state.py:2440: error: Argument "targetid" to "GetReactionsRequest" has incompatible type "int | _ReadOnlyProto[int]"; expected "int"  [arg-type]dd-trace-py (https://github.com/DataDog/dd-trace-py)+ ddtrace/internal/wrapping/__init__.py:301: error: Unused "type: ignore" comment  [unused-ignore]

@ilevkivskyi
Copy link
MemberAuthor

OK, this question reminded me a discussion we had a while ago about what to do for variables annotated withCallable, when should we bind self in them? So I essentially re-use the same logic we agreed on that (which is btw essentially the same story as descriptors are involved under the hood there).

I am happy with the primer, so I will be merging this today/tomorrow unless there are objections.

@ilevkivskyiilevkivskyi merged commitdf9ddfc intopython:masterMar 24, 2025
18 checks passed
@ilevkivskyiilevkivskyi deleted the move-descriptor-set branchMarch 24, 2025 20:09
jhance pushed a commit that referenced this pull requestMay 8, 2025
This is not a pure refactoring, but almost. Right now we are in a weirdsituation where we have two inconsistencies:* `__set__()` is handled in `checker.py` while `__get__()` is handled in`checkmember.py`* rules for when to use binder are slightly different betweendescriptors and settable properties.This PR fixes these two things. As a nice bonus we should get freesupport for unions in `__set__()`.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@JukkaLJukkaLAwaiting requested review from JukkaL

@hauntsaninjahauntsaninjaAwaiting requested review from hauntsaninja

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ilevkivskyi
[8]ページ先頭
©2009-2025 Movatter.jp