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

Number protection #10

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

PyJay
Copy link
Contributor

@PyJay PyJay commented Nov 16, 2020

No description provided.

@gabrielcnr
Copy link
Owner

Thanks for submitting this Pull Request, @PyJay .
Can you please describe briefly what is the issue you are trying to solve here, or the new functionality you're trying to add?

@PyJay
Copy link
Contributor Author

PyJay commented Nov 16, 2020

@gabrielcnr
This merged pull request bypassed using a lambda: None as a callable and 'cheated' by using a dict instead to make tests in Python 2.7 pass

Today I realised that this also had an issue as len(dict) was raising a TypeError which was not being caught at the top function (ls) level. PR9 intends to fix this.

I was not sure whether Python 2.7 should be continued to be supported, however, I got curious about the initial problem and looked into it.

Turns out the lambda: None attribute breaks down by iter_ls to a 1L in python 2.7 (I haven't looked into the exact stacktrace that gets it to this point - I just checked at the 100th call of iter_ls)

>>> one = 1L
>>> one.denominator
1L
>>> id(one) == id(one.denominator)
False

As you can see, it looks like 1L is not interned and this is causing an infinite loop as the denominator (and numerator) of 1L is also 1L. This PR attempts to fix this using special treatment for numbers...

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.

None yet

2 participants