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

[ray client] use is / is not on type check #46464

Merged
merged 1 commit into from
Jul 7, 2024
Merged

Conversation

aslonnie
Copy link
Collaborator

@aslonnie aslonnie commented Jul 7, 2024

rather than using == or !=

rather than using == or !=

Signed-off-by: Lonnie Liu <[email protected]>
@aslonnie aslonnie requested review from jjyao and rynewang July 7, 2024 02:13
@aslonnie aslonnie added the go add ONLY when ready to merge, run all tests label Jul 7, 2024
@@ -96,7 +96,7 @@ def persistent_id(self, obj):
elif isinstance(obj, ClientRemoteFunc):
if obj._ref is None:
obj._ensure_ref()
if type(obj._ref) == InProgressSentinel:
if type(obj._ref) is InProgressSentinel:
return PickleStub(
Copy link

@kevbanawis kevbanawis Jul 7, 2024

Choose a reason for hiding this comment

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

Maybe we can use isinstance(ob._refj, InProgressSentinel) here?
Its more concise, readable, and faster?

https://switowski.com/blog/type-vs-isinstance/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jjyao ? I am just fixing the lint..

instance will also allow sub class instances. I am not sure if it is safe for this case.

@aslonnie aslonnie merged commit 3907a3b into master Jul 7, 2024
5 checks passed
@aslonnie aslonnie deleted the lonnie-240706-lint2 branch July 7, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants