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

Add__hash__ method forpermissions.OperandHolder class#9417

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
auvipy merged 1 commit intoencode:masterfromvanya909:patch-1
Jun 10, 2024

Conversation

vanya909
Copy link
Contributor

@vanya909vanya909 commentedMay 30, 2024•
edited
Loading

OperandHolder is not hashable, so need to add__hash__ method

Description

OperandHolder is not hashable after#8710, need to add__hash__ method to it to provide convenient syntax to filter unique permissions usingset ordict.fromkeys

Ways to represent

1.fromrest_frameworkimportpermissions2.set([permissions.IsAuthenticated&permissions.IsAdminUser])

image

Fixes

  • Remove__eq__ method fromOperandHolder
    or
  • Provide__hash__ method toOperandHolder

Second way is provided in this PR

OttoAndrey, MrShirokow, and TheSuperiorStanislav reacted with thumbs up emoji
`OperandHolder` is not hashable, so need to add `__hash__` method
@auvipyauvipy requested a review froma teamJune 4, 2024 14:02
@auvipyauvipy merged commitfe92f0d intoencode:masterJun 10, 2024
Copy link
Contributor

@peterthomassenpeterthomassen left a comment

Choose a reason for hiding this comment

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

lgtm, sorry being slow.

@auvipy
Copy link
Member

thanks anyway. merged as it was small, after cross checking

@adamchainz
Copy link
Contributor

We should really have tests for this.@vanya909 can you make a follow-up PR with a test that hashes an instance?

auvipy reacted with thumbs up emojivanya909 reacted with eyes emoji

@tomchristie
Copy link
Member

Can you explain why usingset(...) on these classes is useful?

@vanya909
Copy link
ContributorAuthor

@tomchristie

Can you explain why using set(...) on these classes is useful?

It's useful in cases when we want to filter out unique permissions in order to avoid calculating of some complex permissions two or more times

Of course it can be done by just iterating over permissions list and appending in a result list those permissions which haven't been added yet, but usingset() is more simple

In case when we want to keep original order of permissionsdict.fromkeys() can be used, I would saydict.fromkeys() is preferable, but it also requires all permissions to be hashable

@peterthomassen
Copy link
Contributor

peterthomassen commentedJun 16, 2024•
edited
Loading

Also, this fixes the regression introduced by#8710, so don't think it requires justification beyond that

auvipy reacted with thumbs up emoji

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

@peterthomassenpeterthomassenpeterthomassen left review comments

@auvipyauvipyauvipy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@vanya909@auvipy@adamchainz@tomchristie@peterthomassen
[8]ページ先頭
©2009-2025 Movatter.jp