-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
[Style]: flake8 violation E721 #48084
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Cheyu Wu <[email protected]>
Signed-off-by: Cheyu Wu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM, but we should not use isinstance
when we need to check two instances are strictly type equal.
cc @rynewang
…e strictly type equal Signed-off-by: Cheyu Wu <[email protected]>
assert type(a) is type(b), (type(a), type(b)) | ||
assert type(a[0]) is type(b[0]), (type(a[0]), type(b[0])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found that for rule E721, the behavior of old and new version of flake8 are different. So maybe add back the # noqa: E721
comment, and then add a TODO comment to say that this # noqa
should be removed after flake8 is upgraded. For example: TODO: Remove noqa after flake8 being upgraded to 7.1.1
Failed CI: https://buildkite.com/ray-project/microcheck/builds/6190#0192a009-91f2-402b-b318-d7a5fdc04a24
Old version of flake8:
New version (7.1.1) of flake8:
@@ -105,7 +105,7 @@ def df_equals(df1, df2): | |||
if isinstance(df1, pandas.DataFrame) and isinstance(df2, pandas.DataFrame): | |||
if (df1.empty and not df2.empty) or (df2.empty and not df1.empty): | |||
assert False, "One of the passed frames is empty, when other isn't" | |||
elif df1.empty and df2.empty and type(df1) != type(df2): | |||
elif df1.empty and df2.empty and type(df1) is not type(df2): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
Why are these changes needed?
While running the pre-commit hook of flake8, the following error occurs if Python version is 3.12. It's because the version of flake8 is too old.
This commit fixes code that violates the E721 rules.
Related issue number
Closes #48058
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.cc @MortalHappiness