Skip to content

(bpo-44221) fix ImportError in same situations #26324

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

Closed
wants to merge 1 commit into from

Conversation

luizoti
Copy link

@luizoti luizoti commented May 24, 2021

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@luizoti

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@luizoti luizoti changed the title update: fix ImportError in hypotheses 44221: fix ImportError in hypotheses May 24, 2021
Copy link
Contributor

@eamanu eamanu left a comment

Choose a reason for hiding this comment

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

Please sign CLA and write the pr title in the form bpo.NNNN: ....

if isinstance(other, TimerHandle):
return self._when < other._when or self.__eq__(other)
return NotImplemented
if self._when < other._when:
Copy link
Contributor

Choose a reason for hiding this comment

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

We're sure that always other is TimeHandle?

@@ -396,7 +393,7 @@ async def create_unix_server(
The return value is a Server object, which can be used to stop
the service.

path is a str, representing a file system path to bind the
path is a str, representing a file systsem path to bind the
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

@@ -649,7 +632,7 @@ def get_event_loop(self):
"""
if (self._local._loop is None and
not self._local._set_called and
threading.current_thread() is threading.main_thread()):
isinstance(threading.current_thread(), threading._MainThread)):
Copy link
Contributor

Choose a reason for hiding this comment

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

I dind't check deeply but, the intention of the original code is check fi current_thread() is the same object of main_thread(). Why are you change it?

@luizoti luizoti changed the title 44221: fix ImportError in hypotheses (bpo-44221) fix ImportError in hypotheses May 30, 2021
@luizoti
Copy link
Author

luizoti commented May 30, 2021

@eamanu, now it's correct?

@domdfcoding
Copy link
Contributor

I don't see how most of the changes relate to the original issue?

@eamanu
Copy link
Contributor

eamanu commented Jun 9, 2021

@domdfcoding you're right, This PR/issue it's a little confuse (for me)

@luizoti luizoti changed the title (bpo-44221) fix ImportError in hypotheses (bpo-44221) fix ImportError in same situations Jul 8, 2021
@luizoti
Copy link
Author

luizoti commented Jul 8, 2021

I hadn't noticed the 'hypotheses', the truth is, 'in some situations'

@luizoti
Copy link
Author

luizoti commented Jul 8, 2021

I'm going to test this out during the day, and I'm going to try to give the correct follow-up to PR.

@luizoti
Copy link
Author

luizoti commented Jul 8, 2021

I performed more tests, initially this correction took effect, but it seems that it was just a mistake, at this moment the problem continues, I will close this PR and if I can actually find a correction I will submit it again.

@luizoti luizoti closed this Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants