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 "raise StopIteration" #10134

Closed
asmeurer opened this Issue Nov 11, 2015 · 31 comments

Comments

Projects
None yet
9 participants
@asmeurer
Copy link
Member

asmeurer commented Nov 11, 2015

See https://www.python.org/dev/peps/pep-0479/. Support for raising StopIteration is going to be removed.

This issue is easy to fix (at least in principle). Just replace raise StopIteration with return.

There is a __future__ import in Python 3.5 but we can't use it because it is not available in earlier versions (and because of the way __future__ imports work they cannot be imported conditionally). We could add a syntactic test to the code quality tests, however.

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Nov 11, 2015

I guess there is also a deprecation warning that is silent by default, but we could turn it into an error when the tests run. That would be better than a syntactic test in the code quality tests.

@SilentFlame

This comment has been minimized.

Copy link

SilentFlame commented Nov 12, 2015

do we just need to replace the raise StopIteration statement to return?

as "raise StopIteration" is present only in two files:

sympy/combinatorics/perm_groups.py
sympy/core/expr.py

do we need to do anything else in this issue.
Regards.

@gxyd

This comment has been minimized.

Copy link
Member

gxyd commented Nov 12, 2015

as "raise StopIteration" is present only in two files:

Here is what i get

gxyd@swap:~/Public/sympy$ git grep 'raise StopIteration'
sympy/combinatorics/perm_groups.py:            raise StopIteration
sympy/combinatorics/perm_groups.py:            raise StopIteration
sympy/combinatorics/perm_groups.py:                    raise StopIteration
sympy/core/expr.py:            raise StopIteration
sympy/core/sympify.py:    ...         raise StopIteration
sympy/core/sympify.py:    ...         raise StopIteration
sympy/ntheory/factor_.py:        raise StopIteration
sympy/ntheory/factor_.py:        raise StopIteration
sympy/ntheory/factor_.py:        raise StopIteration
sympy/ntheory/factor_.py:                raise StopIteration
sympy/ntheory/factor_.py:                raise StopIteration
sympy/ntheory/residue_ntheory.py:                    raise StopIteration
sympy/ntheory/residue_ntheory.py:                    raise StopIteration
sympy/strategies/branch/core.py:                raise StopIteration()
sympy/strategies/branch/core.py:            raise StopIteration()

So i think it is present in these many files.

@SilentFlame

This comment has been minimized.

Copy link

SilentFlame commented Nov 12, 2015

oh, sorry I misunderstood the message of binary file.

so after changing in the respective modules, is it all needed in this issue?
@asmeurer @gxyd

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Nov 12, 2015

There are also subclasses.

$ git grep 'raise.*StopIteration'
sympy/combinatorics/perm_groups.py:            raise StopIteration
sympy/combinatorics/perm_groups.py:            raise StopIteration
sympy/combinatorics/perm_groups.py:                    raise StopIteration
sympy/core/expr.py:            raise StopIteration
sympy/core/sympify.py:    ...         raise StopIteration
sympy/core/sympify.py:    ...         raise StopIteration
sympy/logic/tests/test_inference.py:    raises(StopIteration, lambda: next(result))
sympy/ntheory/factor_.py:    the factorization and raises ``StopIteration``.
sympy/ntheory/factor_.py:        raise StopIteration
sympy/ntheory/factor_.py:        raise StopIteration
sympy/ntheory/factor_.py:        raise StopIteration
sympy/ntheory/factor_.py:                raise StopIteration
sympy/ntheory/factor_.py:                raise StopIteration
sympy/ntheory/residue_ntheory.py:                    raise StopIteration
sympy/ntheory/residue_ntheory.py:                    raise StopIteration
sympy/simplify/sqrtdenest.py:        raise SqrtdenestStopIteration
sympy/simplify/sqrtdenest.py:            raise SqrtdenestStopIteration
sympy/simplify/sqrtdenest.py:        raise SqrtdenestStopIteration
sympy/strategies/branch/core.py:                raise StopIteration()
sympy/strategies/branch/core.py:            raise StopIteration()
@SilentFlame

This comment has been minimized.

Copy link

SilentFlame commented Nov 12, 2015

@asmeurer

what should be done with these there subclasses:

sympy/simplify/sqrtdenest.py:        raise SqrtdenestStopIteration
sympy/simplify/sqrtdenest.py:            raise SqrtdenestStopIteration
sympy/simplify/sqrtdenest.py:        raise SqrtdenestStopIteration

as replacing the StopIteration here with return doesn't make any sense.

and again do we need to replace this all only in this issue.

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Nov 13, 2015

I don't know. What is the point of that subclass?

@SilentFlame

This comment has been minimized.

Copy link

SilentFlame commented Nov 13, 2015

so should I make a pull request without changing the raise SqrtdenestStopIteration in the modules.

@asmeurer

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Nov 13, 2015

That would be a good start.

@SilentFlame

This comment has been minimized.

Copy link

SilentFlame commented Nov 16, 2015

@asmeurer is this the all until now we need to change here regarding this issue.
have a look on the pull request I made.

@Curious72

This comment has been minimized.

Copy link
Contributor

Curious72 commented Nov 17, 2015

I was looking through the file sympy/build/lib.linux-x86_64-2.7/sympy/logic/tests/test_inference.py
and it has the following lines:

result = satisfiable(A ^ B, all_models=True)
print type(result)
models.remove(next(result))
models.remove(next(result))
raises(StopIteration, lambda: next(result))

the satisfiable is returning a generator named "result"
the use of next (result) requires the StopIteration exception to halt ,
the raises function checks if the lambda expression is raising StopIteration or not , if it doesnt it raises AssertionError, that means it is just there to check if the "result" generator is exhausted or not , Is there any other way to check if the generator has exhausted apart from waiting till it raises StopIteration?

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Nov 17, 2015

I believe that test is correct. return in an iterator is automatically converted to raise StopIteration internally:

In [46]: def test():
   ....:     yield
   ....:     return
   ....:

In [47]: t = test()

In [48]: next(t)

In [49]: next(t)
---------------------------------------------------------------------------
StopIteration                             Traceback (most recent call last)
<ipython-input-49-9494367a8bed> in <module>()
----> 1 next(t)

StopIteration:
@Curious72

This comment has been minimized.

Copy link
Contributor

Curious72 commented Nov 17, 2015

So U cant remove the use of StopIteration as if we use an iterator and use the next() method , it will internally raise the StopIteration exception, So the way to remove this issue would be to abandon the use of next() , or writing customized next() method !!

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Nov 17, 2015

Only "raise StopIteration" should be removed. Other uses of StopIteration (catching the exception) are fine and correct.

@SilentFlame

This comment has been minimized.

Copy link

SilentFlame commented Nov 17, 2015

@asmeurer
there's got a tag on the PR:#10138 I had created saying PR: author's turn what does it mean here.
sorry to ask that basic question but I really am unaware of that tag.

@gxyd

This comment has been minimized.

Copy link
Member

gxyd commented Nov 18, 2015

It just means that: your Pull Request currently needs more work from your side, since the last time someone reviewed. You can continue to work on that PR(without any problem), that tag (author's turn) is helpful for the reviewers.

You can read more about it here

@Curious72

This comment has been minimized.

Copy link
Contributor

Curious72 commented Nov 18, 2015

@asmeurer Dont u think we would have to change more files , as If we replace "raise StopIteration" with return in a method in a file, if any other file imports that method and depends on the StopIteration exception , then we would have to make changes to that file as well !

@Curious72

This comment has been minimized.

Copy link
Contributor

Curious72 commented Nov 18, 2015

hey can u please explain , when we are running tests ,for ex-> /sympy/core/expr.py[3].......[ok]
what does that [ no. ] mean after the filename...
and how to debug the errors using the data on travis , like we can see which files failed

@gxyd

This comment has been minimized.

Copy link
Member

gxyd commented Nov 18, 2015

what does that [ no. ] mean after the filename...

That represents the number of functions or test functions in the file sympy/core/expr/tests/test_core.py. So [3] means there are 3 functions in the file sympy/core/tests/test_core.py. This numbering only appears during running tests, so it will only have a path something like sympy/directory_module/tests/test_file_name.py.

how to debug the errors using the data on travis

This question does not make sense to me. Perhaps you want to know that how to pass the tests that fail in your branch?.

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Nov 18, 2015

@Curious72 only the ability to raise StopIteration is going away, because return is exactly the same as it, but less error prone (see the PEP). The exception itself is still used by the interpreter to signal that a generator has stopped.

@madhur08modi

This comment has been minimized.

Copy link

madhur08modi commented Dec 14, 2015

@asmeurer I want to work on this issue. I replaced raise StopIteration with return statement then tried to run the tests some of tests were unable to pass so can u explain what can be the probable cause for this.

@kshitij10496

This comment has been minimized.

Copy link
Member

kshitij10496 commented Dec 20, 2015

@madhur08modi Can you open a PR for this issue ?

@ashutoshsaboo

This comment has been minimized.

Copy link
Contributor

ashutoshsaboo commented Feb 14, 2016

@asmeurer @jksuom @smichr
Hi. I have been trying to look into this issue.

This is what I got when I searched for just StopIteration -:

ashutoshsaboo@ASHUTOSH-PC:~/SymPyMain/sympy$ git grep 'StopIteration'
doc/ext/docscrape.py:                yield StopIteration
sympy/combinatorics/perm_groups.py:            raise StopIteration
sympy/combinatorics/perm_groups.py:            raise StopIteration
sympy/combinatorics/perm_groups.py:                    raise StopIteration
sympy/core/expr.py:            raise StopIteration
sympy/core/sympify.py:    ...         raise StopIteration
sympy/core/sympify.py:    ...         raise StopIteration
sympy/logic/algorithms/dpll2.py:    except StopIteration:
sympy/logic/algorithms/dpll2.py:    except StopIteration:
sympy/logic/algorithms/dpll2.py:        ... except StopIteration:
sympy/logic/algorithms/dpll2.py:        ... except StopIteration:
sympy/logic/tests/test_inference.py:    raises(StopIteration, lambda: next(result))
sympy/ntheory/factor_.py:    the factorization and raises ``StopIteration``.
sympy/ntheory/factor_.py:        raise StopIteration
sympy/ntheory/factor_.py:        raise StopIteration
sympy/ntheory/factor_.py:        raise StopIteration
sympy/ntheory/factor_.py:                raise StopIteration
sympy/ntheory/factor_.py:    except StopIteration:
sympy/ntheory/factor_.py:                raise StopIteration
sympy/ntheory/factor_.py:        except StopIteration:
sympy/ntheory/residue_ntheory.py:            except StopIteration:
sympy/ntheory/residue_ntheory.py:    except StopIteration:
sympy/ntheory/residue_ntheory.py:                    raise StopIteration
sympy/ntheory/residue_ntheory.py:                    raise StopIteration
sympy/parsing/sympy_tokenize.py:    can be a callable function terminating with StopIteration::
sympy/parsing/sympy_tokenize.py:        except StopIteration:
sympy/polys/distributedmodules.py:        except StopIteration:
sympy/polys/distributedmodules.py:    except StopIteration:
sympy/polys/polyroots.py:    except StopIteration:
sympy/polys/polyroots.py:                except StopIteration:
sympy/sets/sets.py:                except StopIteration:
sympy/simplify/cse_main.py:            except StopIteration:
sympy/simplify/sqrtdenest.py:class SqrtdenestStopIteration(StopIteration):
sympy/simplify/sqrtdenest.py:                    except SqrtdenestStopIteration:
sympy/simplify/sqrtdenest.py:    throws SqrtdenestStopIteration
sympy/simplify/sqrtdenest.py:        raise SqrtdenestStopIteration
sympy/simplify/sqrtdenest.py:            raise SqrtdenestStopIteration
sympy/simplify/sqrtdenest.py:        raise SqrtdenestStopIteration
sympy/solvers/diophantine.py:            except StopIteration:
sympy/solvers/diophantine.py:    partitions are over, the last `next()` call throws the ``StopIteration``
sympy/solvers/diophantine.py:    Here `n` is a non-negative integer. StopIteration exception is raised after
sympy/solvers/tests/test_diophantine.py:            except StopIteration:
sympy/solvers/tests/test_diophantine.py:                except StopIteration:
sympy/solvers/tests/test_diophantine.py:            except StopIteration:
sympy/strategies/branch/core.py:                raise StopIteration()
sympy/strategies/branch/core.py:            raise StopIteration()
sympy/utilities/iterables.py:    except StopIteration:
sympy/utilities/iterables.py:        except StopIteration:

Now, there are several files, if you notice above, that have except StopIteration , which may depend on the output of other modules. as in that if those modules raise StopIteration then only it will not go inside this condition of except StopIteration . Now, if we replace all the raise StopIteration by return() then, as I said above, it will never go in the except StopIteration condition, which may give inaccurate output in some special cases.

And I feel, we need to change the except StopIteration as well to something else, in terms of return because, of the upcoming deprecation of StopIteration ?

So, will return() function automatically get converted to raise StopIteration() internally, so that, the above problem doesn't come into existence?

Another thing, in many tests, the test file checks for raises(StopIteration) . Now, will that cause an issue, once we replace them with return?

Just wanted to get these two things clarified ? @asmeurer @jksuom @smichr
I have been looking into this, and will push a PR, with the changed code after that soon.

Thanks! 😃

@jksuom

This comment has been minimized.

Copy link
Member

jksuom commented Feb 14, 2016

It seems to me that not all raise StopIteration statements need or should be changed. Those in factor_.py, for example, are used to internally control the flow together with except StopIteration. They are not used in generators and could be replaced with a purely locally defined exception such as SymPyStopIteration (though I don't think that would be necessary).

@ashutoshsaboo

This comment has been minimized.

Copy link
Contributor

ashutoshsaboo commented Feb 14, 2016

@jksuom Thanks for your input. 😄

Hmm... So, do you mean to say that, if any StopIteration statements are to be changed, then they are only the one's that involve raise StopIteration (also not all of them)? And that the rest all, having yield StopIteration or except StopIteration or other such one's are surely not to be changed?

And what about the StopIteration statements in the test files? If a test file checks for raises(StopIteration) , then would it fail that test, because it's only returning None now, and not raising StopIteration ? Or will the return statement internally automatically get converted to raise StopIteration (I am not aware about that, hence asking about the same) ? How do we solve that?

Thanks! @jksuom @asmeurer

@jksuom

This comment has been minimized.

Copy link
Member

jksuom commented Feb 14, 2016

Finally, the proposal also clears up the confusion about how to terminate a generator: the proper way is return , not raise StopIteration .

This is from PEP-479. It says that only generators will be affected. Elsewhere there is no need to change raise StopIteration to return (or maybe nothing at the end of a generator function). So it will suffice to find those statements that are in generators.

Or will the return statement internally automatically get converted to raise StopIteration

Apparently returning None will have the same effect internally in the future Python interpreter as the one currently caused by raise StopIteration.

@ashutoshsaboo

This comment has been minimized.

Copy link
Contributor

ashutoshsaboo commented Feb 14, 2016

Hmm I see @jksuom . Thanks for your help.

I looked into all the files having StopIteration , and I found only these files, in which raise StopIteration wasn't being used internally, and could be easily changed without any issues!

Please check out my PR, and let me know in case if there are any changes in the code, that you could tell me!

Thanks ! 😃 @jksuom @asmeurer

@asmeurer asmeurer reopened this Sep 13, 2016

@asmeurer asmeurer added this to the 1.1 milestone Sep 13, 2016

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Sep 13, 2016

There are several instances of this. In Python 3.6, this will raise a DeprecationWarning, causing it to fail in our test suite:

______________________________________________________ sympy/ntheory/tests/test_residue.py:test_residue ______________________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/ntheory/tests/test_residue.py", line 62, in test_residue
    assert [j for j in range(14) if is_quad_residue(j, 14)] == \
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/ntheory/tests/test_residue.py", line 62, in <listcomp>
    assert [j for j in range(14) if is_quad_residue(j, 14)] == \
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/ntheory/residue_ntheory.py", line 610, in is_quad_residue
    r = sqrt_mod(a, p)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/ntheory/residue_ntheory.py", line 250, in sqrt_mod
    r = next(it)
DeprecationWarning: generator 'sqrt_mod_iter' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
_________________________________________ sympy/solvers/tests/test_diophantine.py:test_quadratic_non_perfect_square __________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/solvers/tests/test_diophantine.py", line 158, in test_quadratic_non_perfect_square
    assert check_solutions(x**2 - x*y - y**2 - 3*y)
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/solvers/tests/test_diophantine.py", line 693, in check_solutions
    s = diophantine(eq)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 199, in diophantine
    solution = diop_solve(base, param)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 306, in diop_solve
    return _diop_quadratic(var, coeff, param)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 922, in _diop_quadratic
    solns_pell = diop_DN(D, N)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 1148, in diop_DN
    zs = sqrt_mod(D, abs(m), all_roots=True)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/ntheory/residue_ntheory.py", line 246, in sqrt_mod
    return sorted(list(sqrt_mod_iter(a, p)))
DeprecationWarning: generator 'sqrt_mod_iter' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
__________________________________________________ sympy/solvers/tests/test_diophantine.py:test_issue_9106 ___________________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/solvers/tests/test_diophantine.py", line 165, in test_issue_9106
    for sol in diophantine(eq):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 199, in diophantine
    solution = diop_solve(base, param)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 306, in diop_solve
    return _diop_quadratic(var, coeff, param)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 922, in _diop_quadratic
    solns_pell = diop_DN(D, N)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 1148, in diop_DN
    zs = sqrt_mod(D, abs(m), all_roots=True)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/ntheory/residue_ntheory.py", line 246, in sqrt_mod
    return sorted(list(sqrt_mod_iter(a, p)))
DeprecationWarning: generator 'sqrt_mod_iter' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
______________________________________________________ sympy/solvers/tests/test_diophantine.py:test_DN _______________________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/solvers/tests/test_diophantine.py", line 237, in test_DN
    assert diop_DN(13, 25) == [(3245, 900)]
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 1148, in diop_DN
    zs = sqrt_mod(D, abs(m), all_roots=True)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/ntheory/residue_ntheory.py", line 246, in sqrt_mod
    return sorted(list(sqrt_mod_iter(a, p)))
DeprecationWarning: generator 'sqrt_mod_iter' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
_________________________________________ sympy/solvers/tests/test_diophantine.py:test_diop_ternary_quadratic_normal _________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/solvers/tests/test_diophantine.py", line 350, in test_diop_ternary_quadratic_normal
    assert check_solutions(6*x**2 - y**2 + 10*z**2)
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/solvers/tests/test_diophantine.py", line 693, in check_solutions
    s = diophantine(eq)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 199, in diophantine
    solution = diop_solve(base, param)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 314, in diop_solve
    x_0, y_0, z_0 = _diop_ternary_quadratic_normal(var, coeff)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 2084, in _diop_ternary_quadratic_normal
    sqrt_mod(-a_2*b_2, c_2) is None):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/ntheory/residue_ntheory.py", line 250, in sqrt_mod
    r = next(it)
DeprecationWarning: generator 'sqrt_mod_iter' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
____________________________________________ sympy/solvers/tests/test_diophantine.py:test_diop_ternary_quadratic _____________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/solvers/tests/test_diophantine.py", line 394, in test_diop_ternary_quadratic
    assert check_solutions(x**2 + 3*y**2 + z**2 - x*y - 17*y*z)
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/solvers/tests/test_diophantine.py", line 693, in check_solutions
    s = diophantine(eq)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 199, in diophantine
    solution = diop_solve(base, param)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 309, in diop_solve
    x_0, y_0, z_0 = _diop_ternary_quadratic(var, coeff)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 1805, in _diop_ternary_quadratic
    x_0, y_0, z_0 = _diop_ternary_quadratic(var, _coeff)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 1832, in _diop_ternary_quadratic
    y_0, x_0, z_0 = _diop_ternary_quadratic(var, coeff)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 1805, in _diop_ternary_quadratic
    x_0, y_0, z_0 = _diop_ternary_quadratic(var, _coeff)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 1836, in _diop_ternary_quadratic
    x_0, y_0, z_0 = _diop_ternary_quadratic_normal(var, coeff)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 2084, in _diop_ternary_quadratic_normal
    sqrt_mod(-a_2*b_2, c_2) is None):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/ntheory/residue_ntheory.py", line 250, in sqrt_mod
    r = next(it)
DeprecationWarning: generator 'sqrt_mod_iter' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
____________________________________________________ sympy/solvers/tests/test_diophantine.py:test_descent ____________________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/solvers/tests/test_diophantine.py", line 466, in test_descent
    raises(TypeError, lambda: descent(4, 3))
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/utilities/pytest.py", line 78, in raises
    code()
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/solvers/tests/test_diophantine.py", line 466, in <lambda>
    raises(TypeError, lambda: descent(4, 3))
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 2311, in descent
    x, y, z = descent(B, A)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 2324, in descent
    w = sqrt_mod(A, B)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/ntheory/residue_ntheory.py", line 250, in sqrt_mod
    r = next(it)
DeprecationWarning: generator 'sqrt_mod_iter' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
_________________________________________________ sympy/solvers/tests/test_diophantine.py:test_diopcoverage __________________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/solvers/tests/test_diophantine.py", line 724, in test_diopcoverage
    assert cornacchia(1, 1, 20) is None
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 1221, in cornacchia
    v = sqrt_mod(-b*a1, m, all_roots=True)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/ntheory/residue_ntheory.py", line 246, in sqrt_mod
    return sorted(list(sqrt_mod_iter(a, p)))
DeprecationWarning: generator 'sqrt_mod_iter' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
__________________________________________________ sympy/solvers/tests/test_diophantine.py:test_issue_8943 ___________________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/solvers/tests/test_diophantine.py", line 788, in test_issue_8943
    (3*(x**2 + y**2 + z**2) - 14*(x*y + y*z + z*x))) == \
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 199, in diophantine
    solution = diop_solve(base, param)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 309, in diop_solve
    x_0, y_0, z_0 = _diop_ternary_quadratic(var, coeff)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 1805, in _diop_ternary_quadratic
    x_0, y_0, z_0 = _diop_ternary_quadratic(var, _coeff)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 1832, in _diop_ternary_quadratic
    y_0, x_0, z_0 = _diop_ternary_quadratic(var, coeff)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 1805, in _diop_ternary_quadratic
    x_0, y_0, z_0 = _diop_ternary_quadratic(var, _coeff)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 1836, in _diop_ternary_quadratic
    x_0, y_0, z_0 = _diop_ternary_quadratic_normal(var, coeff)
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/solvers/diophantine.py", line 2084, in _diop_ternary_quadratic_normal
    sqrt_mod(-a_2*b_2, c_2) is None):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/ntheory/residue_ntheory.py", line 250, in sqrt_mod
    r = next(it)
DeprecationWarning: generator 'sqrt_mod_iter' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
___________________________________________________ sympy/strategies/branch/tests/test_core.py:test_chain ____________________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/strategies/branch/tests/test_core.py", line 77, in test_chain
    assert list(chain(inc, inc)(2)) == [4]
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
DeprecationWarning: generator 'chain.<locals>.chain_brl' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
_________________________________________________ sympy/strategies/branch/tests/test_tools.py:test_zero_ints _________________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/strategies/branch/tests/test_tools.py", line 28, in test_zero_ints
    assert set(brl(expr)) == expected
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 13, in exhaust_brl
    for nexpr in brule(expr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 47, in multiplex_brl
    for nexpr in brl(expr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 101, in chain_brl
    for nexpr in head(expr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/traverse.py", line 23, in all_rl
    argss = product(*map(brule, children(expr)))
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
DeprecationWarning: generator 'chain.<locals>.chain_brl' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
__________________________________________________ sympy/strategies/branch/tests/test_tools.py:test_split5 ___________________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/strategies/branch/tests/test_tools.py", line 36, in test_split5
    assert set(brl(expr)) == expected
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 13, in exhaust_brl
    for nexpr in brule(expr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 47, in multiplex_brl
    for nexpr in brl(expr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 101, in chain_brl
    for nexpr in head(expr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/traverse.py", line 23, in all_rl
    argss = product(*map(brule, children(expr)))
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
DeprecationWarning: generator 'chain.<locals>.chain_brl' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
_____________________________________________ sympy/strategies/branch/tests/test_traverse.py:test_top_down_easy ______________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/strategies/branch/tests/test_traverse.py", line 14, in test_top_down_easy
    assert set(brl(expr)) == {expected}
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 101, in chain_brl
    for nexpr in head(expr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/traverse.py", line 23, in all_rl
    argss = product(*map(brule, children(expr)))
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
DeprecationWarning: generator 'chain.<locals>.chain_brl' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
___________________________________________ sympy/strategies/branch/tests/test_traverse.py:test_top_down_big_tree ____________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/strategies/branch/tests/test_traverse.py", line 21, in test_top_down_big_tree
    assert set(brl(expr)) == {expected}
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 101, in chain_brl
    for nexpr in head(expr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/traverse.py", line 23, in all_rl
    argss = product(*map(brule, children(expr)))
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
DeprecationWarning: generator 'chain.<locals>.chain_brl' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
________________________________________ sympy/strategies/branch/tests/test_traverse.py:test_top_down_harder_function ________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/strategies/branch/tests/test_traverse.py", line 33, in test_top_down_harder_function
    assert set(brl(expr)) == expected
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 101, in chain_brl
    for nexpr in head(expr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/traverse.py", line 23, in all_rl
    argss = product(*map(brule, children(expr)))
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 101, in chain_brl
    for nexpr in head(expr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/traverse.py", line 23, in all_rl
    argss = product(*map(brule, children(expr)))
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
DeprecationWarning: generator 'chain.<locals>.chain_brl' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
__________________________________________________ sympy/strategies/branch/tests/test_traverse.py:test_sall __________________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/strategies/branch/tests/test_traverse.py", line 46, in test_sall
    assert list(brl(expr)) == [expected]
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/traverse.py", line 23, in all_rl
    argss = product(*map(brule, children(expr)))
DeprecationWarning: generator 'do_one.<locals>.do_one_brl' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
____________________________________________________ sympy/strategies/tests/test_tree.py:test_allresults _____________________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/strategies/tests/test_tree.py", line 65, in test_allresults
    assert set(allresults((inc, dec))(3)) == {3}
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
DeprecationWarning: generator 'chain.<locals>.chain_brl' raised StopIteration
______________________________________________________________________________________________________________________________________________________________
_______________________________________________________ sympy/strategies/tests/test_tree.py:test_brute _______________________________________________________
  File "/users/aaronmeurer/documents/python/sympy/sympy/sympy/strategies/tests/test_tree.py", line 75, in test_brute
    assert fn(2) == (2 + 1)**2
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/tree.py", line 134, in <lambda>
    return lambda expr: min(tuple(allresults(tree, **kwargs)(expr)),
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
  File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/strategies/branch/core.py", line 102, in chain_brl
    for nnexpr in chain(*tail)(nexpr):
DeprecationWarning: generator 'chain.<locals>.chain_brl' raised StopIteration

@asmeurer asmeurer referenced this issue Sep 13, 2016

Closed

Python 3.6 support #11609

2 of 5 tasks complete
@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Sep 13, 2016

And from the doctests:

sympy/core/sympify.py[4] ../Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/matrices/matrices.py:358: DeprecationWarning: generator 'MyList1.__iter__' raised StopIteration
  for row in args[0]:
/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/matrices/matrices.py:358: DeprecationWarning: generator 'MyList2.__iter__' raised StopIteration
  for row in args[0]:
sympy/doc/src/modules/simplify/hyperexpand.rst [18]                                                                                                       [OK]
/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/ntheory/residue_ntheory.py:246: DeprecationWarning: generator 'sqrt_mod_iter' raised StopIteration
  return sorted(list(sqrt_mod_iter(a, p)))
/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/ntheory/residue_ntheory.py:250: DeprecationWarning: generator 'sqrt_mod_iter' raised StopIteration
  r = next(it)

There is an additional issue, which is that deprecation warnings do not cause the doctest to fail. See #11610.

@jksuom

This comment has been minimized.

Copy link
Member

jksuom commented Sep 19, 2016

These are the remaining occurrences of raise StopIteration:

sympy/core/sympify.py:    ...         raise StopIteration
sympy/core/sympify.py:    ...         raise StopIteration
sympy/ntheory/factor_.py:        raise StopIteration
sympy/ntheory/factor_.py:        raise StopIteration
sympy/ntheory/factor_.py:        raise StopIteration
sympy/ntheory/factor_.py:                raise StopIteration
sympy/ntheory/factor_.py:                raise StopIteration
sympy/ntheory/residue_ntheory.py:                    raise StopIteration
sympy/ntheory/residue_ntheory.py:                    raise StopIteration
sympy/strategies/branch/core.py:                raise StopIteration()
sympy/strategies/branch/core.py:            raise StopIteration()

It seems that they should be replaced with return except those in factor_.py, which do not appear in generators. In residue_ntheory.py, sqrt_mod should be modified to catch the return value None of the generator returned by sqrt_mod_iter. The following could be a first step.

@@ -248,6 +248,8 @@ def sqrt_mod(a, p, all_roots=False):
         p = abs(as_int(p))
         it = sqrt_mod_iter(a, p)
         r = next(it)
+        if r is None:
+            return
         if r > p // 2:
             return p - r
         elif r < p // 2:
@@ -255,6 +257,8 @@ def sqrt_mod(a, p, all_roots=False):
         else:
             try:
                 r = next(it)
+                if r is None:
+                    return
                 if r > p // 2:
                     return p - r
             except StopIteration:

Then the code should be simplified by removing the useless try blocks.

@spapanik

This comment has been minimized.

Copy link
Contributor

spapanik commented Sep 19, 2016

I am working on it. Even when sqrt_mod_iter will be changed from using StopIteration to bare return, it will still raise a StopIteration to catch, so I don't think that the above change will not work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment