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

Add no-subclass-builtin check #324

Merged
merged 1 commit into from
Jan 31, 2024
Merged

Add no-subclass-builtin check #324

merged 1 commit into from
Jan 31, 2024

Conversation

dosisod
Copy link
Owner

@dosisod dosisod commented Jan 31, 2024

Closes #323.

I am disabling this check by default for 2 reasons:

  • isinstance(x, dict) fails for UserDict objects (including list/str)
  • Changing dict to UserDict is not a quick fix, and you will probably have to change your class definition to get the most out of UserDict (ie, removing reimplemented methods, something Refurb can't easily detect).

Teams that want to enforce this can enable this option, but overall, it is hard to deduce what is/is not proper usage of classes subclassing dict.

Closes #323.

I am disabling this check by default for 2 reasons:

* `isinstance(x, dict)` fails for `UserDict` objects (including `list`/`str`)
* Changing `dict` to `UserDict` is not a quick fix, and you will probably have
  to change your class definition to get the most out of `UserDict` (ie,
  removing reimplemented methods, something Refurb can't easily detect).

Teams that want to enforce this can enable this option, but overall, it is hard
to deduce what is/is not proper usage of classes subclassing `dict`.
@dosisod dosisod merged commit d72ecfd into master Jan 31, 2024
3 checks passed
@dosisod dosisod deleted the add-no-subclass-builtin branch January 31, 2024 05:58
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.

[Enhancement]: subclassing collections.UserDict over dict
1 participant