Sounds good to me. Can you submit a pull request? Aaron Meurer
On Mon, Nov 11, 2013 at 1:37 PM, Amit Saha <[email protected]> wrote: > On Mon, Nov 11, 2013 at 11:05 PM, Amit Saha <[email protected]> wrote: >> On Mon, Nov 11, 2013 at 9:13 PM, Amit Saha <[email protected]> wrote: >>> Hello, >>> >>> Consider the following: >>> >>> Python 3: >>> >>>>>> from sympy import Line, Point >>>>>> line = Line(Point(1,0), slope = 1) >>>>>> Point.is_collinear(line) >>> True >>> >>> Python 2: >>> >>>>>> from sympy import Line >>>>>> line = Line(Point(1,0), slope = 1) >>>>>> Point.is_collinear(line) >>> Traceback (most recent call last): >>> File "<stdin>", line 1, in <module> >>> TypeError: unbound method is_collinear() must be called with Point >>> instance as first argument (got Line instance instead) >>> >>> In the second case (because of Python 2's concept of unbound methods), >>> the user at least knows that something wrong has happened. In the >>> first case, however, since Python 3 allows class methods to be called >>> with any object instance, it is simply not a problem. >>> >>> I propose the following fix: >>> >>> diff --git a/sympy/geometry/point.py b/sympy/geometry/point.py >>> index 4e941a1..7f359f5 100644 >>> --- a/sympy/geometry/point.py >>> +++ b/sympy/geometry/point.py >>> @@ -151,6 +151,7 @@ def length(self): >>> """ >>> return S.Zero >>> >>> + @classmethod >>> def is_collinear(*points): >>> """Is a sequence of points collinear? >>> >>> @@ -209,6 +210,10 @@ def is_collinear(*points): >>> # Coincident points are irrelevant and can confuse this algorithm. >>> # Use only unique points. >>> points = list(set(points)) >>> + >>> + if not any([isinstance(p, Point) for p in points]): >> >> That should be all() instead. > > Realized that I had missed something else as well. Anyway, here is the > proposed fix: > > diff --git a/sympy/geometry/point.py b/sympy/geometry/point.py > index 4e941a1..c524c61 100644 > --- a/sympy/geometry/point.py > +++ b/sympy/geometry/point.py > @@ -151,6 +151,7 @@ def length(self): > """ > return S.Zero > > + @classmethod > def is_collinear(*points): > """Is a sequence of points collinear? > > @@ -208,7 +209,11 @@ def is_collinear(*points): > """ > # Coincident points are irrelevant and can confuse this algorithm. > # Use only unique points. > - points = list(set(points)) > + points = list(set(points[1:])) > + > + if not all([isinstance(p, Point) for p in points]): > + raise GeometryError('Must pass only point objects') > + > if len(points) == 0: > return False > if len(points) <= 2: > > > > Best, > Amit. > > > -- > http://echorand.me > > -- > You received this message because you are subscribed to the Google Groups > "sympy" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at http://groups.google.com/group/sympy. > For more options, visit https://groups.google.com/groups/opt_out. -- You received this message because you are subscribed to the Google Groups "sympy" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/sympy. For more options, visit https://groups.google.com/groups/opt_out.
