Repository: sympy/sympy
Pull request: Fixes Idx object to accept non-integer bound
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->
Fixes Idx object to accept non-integer bound
#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->
Fixes #18604 

#### Brief description of what is fixed or changed


#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* tensor
  * Fix Idx object to accept non-integer bound
<!-- END RELEASE NOTES -->

Problem statement:
Idx object can accepts non-integer bounds
It is my understanding that this code should not work: the lower and upper bounds should be integers. Instead it runs without any errors:

```
import sympy as sp
m, n = sp.symbols("m, n", real=True)
i = sp.Idx("i", (m, n))
```

Note however that:

```
sp.Idx("i", m)
```

produces the expected result:

```
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-59-166e71a7f3f7> in <module>
----> 1 sp.Idx("i", m)

/usr/local/lib/python3.7/dist-packages/sympy/tensor/indexed.py in __new__(cls, label, range, **kw_args)
    665         elif isinstance(range, Expr):
    666             if not (range.is_integer or range is S.Infinity):
--> 667                 raise TypeError("Idx object requires an integer dimension.")
    668             args = label, Tuple(0, range - 1)
    669         elif range:

TypeError: Idx object requires an integer dimension.
```

The check for integer dimension should be done to both upper and lower bounds when a tuple is given as argument.

Hints from issue discussion:
I think it should be okay to use something that is possibly an integer like a plain `Symbol('n')`. So this is correct:
```julia
In [4]: x = Symbol('x', integer=False)                                                                                                         

In [5]: Idx('i', (x, y))                                                                                                                       
---------------------------------------------------------------------------
TypeError
```
What should be fixed is that this should not raise:
```julia
In [8]: Idx('i', Symbol('x'))                                                                                                                  
---------------------------------------------------------------------------
TypeError
```
The check for `range.is_integer` needs to take account of the case where `range.is_integer` gives None. Perhaps it should use `fuzzy_not/and`.
I am working on this.

Commit message: fix issue 18604

Patch to review:
diff --git a/sympy/tensor/indexed.py b/sympy/tensor/indexed.py
index cde31f71f8c4..8abd3946f3a1 100644
--- a/sympy/tensor/indexed.py
+++ b/sympy/tensor/indexed.py
@@ -111,7 +111,7 @@
 from sympy.core.symbol import _filter_assumptions, Symbol
 from sympy.core.compatibility import (is_sequence, NotIterable,
                                       Iterable)
-from sympy.core.logic import fuzzy_bool
+from sympy.core.logic import fuzzy_bool, fuzzy_and, fuzzy_not
 from sympy.core.sympify import _sympify
 from sympy.functions.special.tensor_functions import KroneckerDelta
 
@@ -663,7 +663,7 @@ def __new__(cls, label, range=None, **kw_args):
                     raise TypeError("Idx object requires integer bounds.")
             args = label, Tuple(*range)
         elif isinstance(range, Expr):
-            if not (range.is_integer or range is S.Infinity):
+            if fuzzy_and([fuzzy_not(range.is_integer), fuzzy_not(range is S.Infinity)]):
                 raise TypeError("Idx object requires an integer dimension.")
             args = label, Tuple(0, range - 1)
         elif range:
diff --git a/sympy/tensor/tests/test_indexed.py b/sympy/tensor/tests/test_indexed.py
index e362e39fdde2..bf93d7f238df 100644
--- a/sympy/tensor/tests/test_indexed.py
+++ b/sympy/tensor/tests/test_indexed.py
@@ -422,6 +422,13 @@ def test_issue_12780():
     raises(TypeError, lambda: i.subs(n, 1.5))
 
 
+def test_issue_18604():
+    m = symbols("m")
+    assert Idx("i", m).name == 'i'
+    assert Idx("i", m).lower == 0
+    assert Idx("i", m).upper == m - 1
+
+
 def test_Subs_with_Indexed():
     A = IndexedBase("A")
     i, j, k = symbols("i,j,k")


Code review comments:
diff_hunk: @@ -663,7 +663,7 @@ def __new__(cls, label, range=None, **kw_args):
                     raise TypeError("Idx object requires integer bounds.")
             args = label, Tuple(*range)
         elif isinstance(range, Expr):
-            if not (range.is_integer or range is S.Infinity):
+            if fuzzy_and([fuzzy_not(range.is_integer), fuzzy_not(range is S.Infinity)]):
path: sympy/tensor/indexed.py
text: @user1:
Since `range is S.Infinity` is a normal bool rather than a fuzzy bool it isn't necessary to use `fuzzy_not` for that so this could be
```
if range is not S.Infinity and fuzzy_not(range.is_integer):
```