Fix #23: Make Field.Access NotIterable

Why?

Sympy does not try to iterate when the object is NotIterable. If it has __iter__ but iter throws it will only sometimes think its not iterable.

changed the description

Damn it! This is so inherently broken that it requires some upstream changes. Probably, that no NotIterable is always excluded.

https://github.com/sympy/sympy/pull/19772/files

Maybe we can backport that version of the function for older Sympy version if the functionality is important.

assigned to @kuron

assigned to @holzer and unassigned @kuron

assigned to @markl and unassigned @holzer

assigned to @holzer and unassigned @markl

@kuron

All test pass with the upstream change.

I don‘t fully understand the problem. Could you add a pure-SymPy testcase to your upstream patch that demonstrates the problem? Am I correct in assuming that there are places inside SymPy that call iterable(..., exclude=...) and forget to add NotIterable to the list of exclusions?

Yes, that's exactly the problem. In most cases people forget to exclude NotIterable.

Throwing TypeError will make iterable always False but other parts of Sympy think Field.Access is iterable because it has __iter__. Python iter works without a __iter__ method by simply using __getitem__[0], __getitem__[1] ...

So in summary there's no way to tell Sympy that something is not iterable.

@hu62bodi enjoy, maybe first look at the issue.

Perhaps you could add that explanation to the SymPy pull request.

I think I found a solution: adding a magic _iterable = False member. Will try that out.

This is our problem

class Test8(NotIterable):
    def __getitem__(self, *idx):
        return "something"


assert iter(Test8())
assert iterable(Test8()) is False
assert iterable(Test8(), exclude=str) is False # will fail without my PR

will add that test to the PR.

Our issue can be solved by

class Test8(NotIterable):
    _iterable = False
    def __getitem__(self, *idx):
        return "something"


assert iter(Test8())
assert iterable(Test8()) is False
assert iterable(Test8(), exclude=str) is False  # correct behavior without this Sympy PR.

added 1 commit

• f047af73 - Fix #23 (closed): Make Field.Access NotIterable

Compare with previous version

added 1 commit

• be52c6e1 - Fix #23 (closed): Make Field.Access NotIterable

Compare with previous version

Maybe _iterable = False should be added to the NotIterable class? That would be a single-line solution to the problem.

This is how it's currently solved: with _iterable =False

I was talking about your Sympy pull request. Instead of modifying def iterable, modify class NotIterable.

Good idea.

@holzer ping. NotIterable is still not respected by SymPy but either _iterable==False or hasattr(., '__iter__) == False.

added 35 commits

• be52c6e1...558b9f81 - 34 commits from branch pycodegen:master
• f4ba5448 - Fix #23 (closed): Make Field.Access NotIterable

Compare with previous version

Ok is this ready to merge or do we need to wait for SymPy to merge your MR?

This is ready to merge. It will work for either mechanism that SymPy uses to check that it's not iterable.

merged

mentioned in commit 04eb30b6

changed the description