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

remove object class from base classes in type calls #731

Closed
wants to merge 24 commits into from
Closed

remove object class from base classes in type calls #731

wants to merge 24 commits into from

Conversation

lev-blit
Copy link

@lev-blit lev-blit commented Oct 8, 2022

This solves #532

@lev-blit lev-blit marked this pull request as draft October 8, 2022 12:22
@lev-blit lev-blit marked this pull request as ready for review October 8, 2022 12:28
pyupgrade/_token_helpers.py Outdated Show resolved Hide resolved
pyupgrade/_plugins/type_bases_object.py Outdated Show resolved Hide resolved
pyupgrade/_token_helpers.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
tests/features/type_bases_object_test.py Show resolved Hide resolved
@lev-blit
Copy link
Author

@asottile what should be done for (foo, object, bar) and similar cases where object isn't first or last? This doesn't seem like something anyone should write, but maybe it should be supported(or on the contrary, explicitly unsupported)

@asottile
Copy link
Owner

so the weird thing is type('a', (), {}) in python 2.x also makes a new-style class so it also seems like something someone shouldn't write too:

>>> x = type('a', (), {})
>>> x.__bases__
(<type 'object'>,)

I would probably remove it anyway 🤷

type_start = find_open_paren(tokens, 0)
bases_start = find_open_paren(tokens, type_start + 1)
bases, end = parse_call_args(tokens, bases_start)
for base_start, base_end in bases: # pragma: no cover
Copy link
Owner

Choose a reason for hiding this comment

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

can't ship with this -- means you haven't tested everything

this function is way too complicated -- I don't think it's correct either. you should also be able to use the original object base index without needing to re-find it here and utilize the argument removal functions


def remove_base_class_from_type_call(i: int, tokens: list[Token]) -> None:
type_start = find_open_paren(tokens, 0)
bases_start = find_open_paren(tokens, type_start + 1)
Copy link
Owner

Choose a reason for hiding this comment

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

this isn't necessarily correct -- type(class_name(), (...))

del tokens[base_start:base_end]
# (foo, bar, ..., object[,]) -> (foo, bar, ...[,])
else:
if tokens[base_end - 1].name == UNIMPORTANT_WS and \
Copy link
Owner

Choose a reason for hiding this comment

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

don't use backslashes

@lev-blit
Copy link
Author

I won't have much time to work on this anymore :/ -- closing the PR so others might take a stab at this

p.s. thank you for being patient with me through this (my first contribution), this was an educating experience :)

@lev-blit lev-blit closed this Oct 31, 2022
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.

2 participants