Repository: sympy/sympy
Pull request: imported filldedent in line.py
fixes https://github.com/sympy/sympy/issues/13705 
@smichr please review
<!-- Please give this pull request a descriptive title. Pull requests with descriptive titles are more likely to receive reviews. Describe what you changed! A title that only references an issue number is not descriptive. -->

<!-- If this pull request fixes an issue please indicate which issue by typing "Fixes #NNNN" below. -->


Problem statement:
import filldedent in line.py
The following should raise a ValueError:
```
>>> Line((x,1),(2,3)).arbitrary_point(x)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "sympy\geometry\line.py", line 236, in arbitrary_point
    raise ValueError(filldedent('''
NameError: global name 'filldedent' is not defined
```
The following mod to line.py should be made and a test of the above added to the place where `arbitrary_point` is tested in the `test_line.py` file.

```diff
diff --git a/sympy/geometry/line.py b/sympy/geometry/line.py
index a542ddb..8ac0dc8 100644
--- a/sympy/geometry/line.py
+++ b/sympy/geometry/line.py
@@ -33,7 +33,7 @@

 from .entity import GeometryEntity, GeometrySet
 from .point import Point, Point3D
-from sympy.utilities.misc import Undecidable
+from sympy.utilities.misc import Undecidable, filldedent


 class LinearEntity(GeometrySet):
```

Hints from issue discussion:
@smichr I would like to work on this issue. I have looked into the arbitrary_point tests and was wondering what would be the best way to write a test for the ValueError, since that error is raised in the arbitrary_point method in the line.py file as shown below
```
if t.name in (f.name for f in self.free_symbols):
            raise ValueError(filldedent('''
                Symbol %s already appears in object
                and cannot be used as a parameter.
                ''' % t.name))
```

Commit message: Merge pull request #1 from TheGluck/filldedent

Adds test

Patch to review:
diff --git a/sympy/geometry/line.py b/sympy/geometry/line.py
index 6af9079a9657..ab2cc8a9e569 100644
--- a/sympy/geometry/line.py
+++ b/sympy/geometry/line.py
@@ -32,7 +32,7 @@
 
 from .entity import GeometryEntity, GeometrySet
 from .point import Point, Point3D
-from sympy.utilities.misc import Undecidable
+from sympy.utilities.misc import Undecidable, filldedent
 
 
 class LinearEntity(GeometrySet):
diff --git a/sympy/geometry/tests/test_line.py b/sympy/geometry/tests/test_line.py
index 2475838189ca..766106a1c182 100644
--- a/sympy/geometry/tests/test_line.py
+++ b/sympy/geometry/tests/test_line.py
@@ -33,7 +33,7 @@ def warn_with_traceback(message, category, filename, lineno, file=None, line=Non
 
 
 warnings.showwarning = warn_with_traceback
-warnings.simplefilter('always', UserWarning)  # make sure to show warnings every time they occurr
+warnings.simplefilter('always', UserWarning)  # make sure to show warnings every time they occur
 
 
 def feq(a, b):
@@ -76,6 +76,12 @@ def test_arbitrary_point():
     assert Segment3D(Point3D(x1, x1, x1), Point3D(y1, y1, y1)).length == sqrt(3) * sqrt((x1 - y1) ** 2)
     assert Segment3D((1, 1, 1), (2, 3, 4)).arbitrary_point() == \
            Point3D(t + 1, 2 * t + 1, 3 * t + 1)
+    try:
+        Line((x,1),(2,3)).arbitrary_point(x)
+    except ValueError:
+        assert True
+    except BaseException:
+        assert False
 
 
 def test_are_concurrent_2d():


Code review comments:
diff_hunk: @@ -76,6 +76,12 @@ def test_arbitrary_point():
     assert Segment3D(Point3D(x1, x1, x1), Point3D(y1, y1, y1)).length == sqrt(3) * sqrt((x1 - y1) ** 2)
     assert Segment3D((1, 1, 1), (2, 3, 4)).arbitrary_point() == \
            Point3D(t + 1, 2 * t + 1, 3 * t + 1)
+    try:
+        Line((x,1),(2,3)).arbitrary_point(x)
+    except ValueError:
+        assert True
+    except BaseException:
+        assert False
path: sympy/geometry/tests/test_line.py
text: @user1:
You can use `raises` from `sympy.utilities.pytest` to test correct error is raised. See `test_raises` around line 649.

@user2:
[BaseException](https://docs.python.org/2/library/exceptions.html#exceptions.BaseException) is only used as a base class for other exceptions.

@user3:
@user1 The code as written is progmatically correct, the test is passed if a ValueError is thrown and fails if any other error is thrown. BaseException is the superclass of all exceptions in python therefore when an exception is inevitably thrown, either it will be a ValueError and will be caught by the first except clause, or it will be caught by the second except clause. This functions as intended.

@user4:
@user2 is telling you how we test such things in the test suite. The `raises` function will do what you are saying with less noise.

@user3:
I see what you mean now, what do you mean by noise though?