Opened 10 years ago

Closed 8 years ago

#13890 closed defect (fixed)

small bug in point3d

Reported by: Timo Jolivet Owned by: jason, was
Priority: major Milestone: sage-6.5
Component: graphics Keywords: plot3d, point3d
Cc: Merged in:
Authors: Frédéric Chapoton Reviewers: Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: 070976c (Commits, GitHub, GitLab) Commit: 070976c3f380640876f96cf9fbb1bc603aa87d36
Dependencies: Stopgaps:

Status badges

Description

The following three commands work as expected (a Graphics object is returned)

sage: points([(1,1)])                                                                                      

sage: points(iter([(1,1)]))                                                                                

sage: points([(1,1,1)])                                                                                    

but the following command raises an AttributeError.

sage: points(iter([(1,1,1)]))
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)

/home/jaje/dropbox/Dropbox/IFS/<ipython console> in <module>()

/home/jaje/sage-5.5/local/lib/python2.7/site-packages/sage/plot/point.pyc in point(points, **kwds)
    310     except (ValueError, TypeError):
    311         from sage.plot.plot3d.shapes2 import point3d
--> 312         return point3d(points, **kwds)
    313 
    314 @rename_keyword(color='rgbcolor', pointsize='size')

/home/jaje/sage-5.5/local/lib/python2.7/site-packages/sage/plot/plot3d/shapes2.pyc in point3d(v, size, **kwds)
   1021     else:
   1022         A = sum([Point(z, size, **kwds) for z in v])
-> 1023         A._set_extra_kwds(kwds)
   1024         return A
   1025 

AttributeError: 'int' object has no attribute '_set_extra_kwds'

Change History (33)

comment:1 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.11sage-5.12

comment:2 Changed 9 years ago by For batch modifications

Milestone: sage-6.1sage-6.2

comment:3 Changed 8 years ago by For batch modifications

Milestone: sage-6.2sage-6.3

comment:4 Changed 8 years ago by For batch modifications

Milestone: sage-6.3sage-6.4

comment:5 Changed 8 years ago by Frédéric Chapoton

Authors: Frédéric Chapoton
Branch: u/chapoton/13890
Commit: 745550226729b06d349c4ba83f6ab457f464d1b8
Keywords: plot3d point3d added
Status: newneeds_review

New commits:

7455502trac #13890 allow use of iterators in point3d

comment:6 Changed 8 years ago by Frédéric Chapoton

Status: needs_reviewneeds_work

comment:7 Changed 8 years ago by git

Commit: 745550226729b06d349c4ba83f6ab457f464d1b852dda634446582b8bed2c36f0ddfe383451d37e6

Branch pushed to git repo; I updated commit sha1. New commits:

52dda63trac #13890 suggestion to use point3d instead for iterator input

comment:8 Changed 8 years ago by git

Commit: 52dda634446582b8bed2c36f0ddfe383451d37e6e042380341e6ccd0f664a0226eb8f3f729b17319

Branch pushed to git repo; I updated commit sha1. New commits:

e042380trac #13890 final touch : case of empty list

comment:9 Changed 8 years ago by Frédéric Chapoton

Status: needs_workneeds_review

New commits:

e042380trac #13890 final touch : case of empty list

comment:10 Changed 8 years ago by Karl-Dieter Crisman

This applies, but apparently needs rebasing since I still get the same error after applying this. There does seem to be some new code in that spot.

Interestingly, n the meantime the original error has changed.

sage: points(iter([(1,1,1)]))
---------------------------------------------------------------------------
   1077 
   1078     """
-> 1079     if len(v) == 3:
   1080         try:
   1081             # check if the first element can be changed to a float

TypeError: object of type 'listiterator' has no len()

By the way, should we try to accommodate generators as well?

points( (x,x,x) for x in range(3) )

or the like.

comment:11 Changed 8 years ago by Karl-Dieter Crisman

Status: needs_reviewneeds_work

comment:12 Changed 8 years ago by git

Commit: e042380341e6ccd0f664a0226eb8f3f729b17319795d88f861c24593000707fe277b3c0c86b06d03

Branch pushed to git repo; I updated commit sha1. New commits:

1e95d9dMerge branch 'u/chapoton/13890' in 6.4.rc2
795d88ftrac #13890 *forbidden* use of iterator

comment:13 Changed 8 years ago by Frédéric Chapoton

Status: needs_workneeds_review

I have now made so that use of iterators is forbidden in points. The reason is that one cannot test if the point has 2 or 3 dimensions without burning the iterator. So it does not make sense to try to guess what is the expected dimension. But iterators are now allowed in both point2d and point3d.

comment:14 Changed 8 years ago by Karl-Dieter Crisman

Aha! I see what happened. Glad I tried that out. I'll take a look at this momentarily.

comment:15 Changed 8 years ago by Karl-Dieter Crisman

Hmm, but look at #10478. There is an explicit example there (oddly, not doctested) for

line(iter([(0,0), (1,0), (2,2)]))

and points and line should really work the same. Can't we just do points = list(points) and then test like they do there?

comment:16 Changed 8 years ago by git

Commit: 795d88f861c24593000707fe277b3c0c86b06d0339fc0177cbe6ad52befd22715d78efd4e86dce1e

Branch pushed to git repo; I updated commit sha1. New commits:

f5e7d23Merge branch 'u/chapoton/13890' into 6.5.b0
39fc017trac #13890 now allowing iterators

comment:17 Changed 8 years ago by Frédéric Chapoton

ping ?

comment:18 Changed 8 years ago by Karl-Dieter Crisman

Sorry, other business (Sage-wise as well) to attend to, and then a break. Is this behavior desired?

sage: point(iter([]))  # works

sage: point2d(iter([]))
---------------------------------------------------------------------------
ValueError             

Actually, probably only the latter (and 3d version thereof) is desired, since one can't know what dimension an empty list should be... or we could default to 2d.

comment:19 Changed 8 years ago by Marc Mezzarobba

Status: needs_reviewneeds_info

comment:20 Changed 8 years ago by git

Commit: 39fc0177cbe6ad52befd22715d78efd4e86dce1ef07d43ce5c9fb71200c014e190e9b697c9c2c861

Branch pushed to git repo; I updated commit sha1. New commits:

ee00aa9Merge branch 'u/chapoton/13890' into 6.5.b6
f07d43ctrac #13890 handling empty iterators (yes we can)

comment:21 Changed 8 years ago by Frédéric Chapoton

Status: needs_infoneeds_review

comment:22 Changed 8 years ago by Karl-Dieter Crisman

So the empty iterator should be 2d, not 3d (or nothing)? Just checking if this is the "right" design decision.

comment:23 Changed 8 years ago by Frédéric Chapoton

Sorry, I do not understand if you still have a concern or not ?

comment:24 Changed 8 years ago by Karl-Dieter Crisman

Well, I'm asking about that. To me point2d([]) and point3d([]) are meaningful; point([]) maybe isn't, because it isn't clear what the default dimension should be. Same for empty iterators. Maybe I'm thinking about it too much.

comment:25 Changed 8 years ago by Frédéric Chapoton

It seems to me that the initial aim of this ticket is met (namely allowing to use iterators in all kinds of points). Maybe this is enough ?

comment:26 Changed 8 years ago by Karl-Dieter Crisman

Reviewers: Karl-Dieter Crisman
Status: needs_reviewpositive_review

Yes, upon further thought you are right, that was a pre-existing thing. Thanks! Sorry this small thing ended up going crazy long.

comment:27 Changed 8 years ago by Karl-Dieter Crisman

Milestone: sage-6.4sage-6.5

comment:28 Changed 8 years ago by Volker Braun

Status: positive_reviewneeds_work

Some doctests fail, e.g.

sage -t --long --warn-long 56.5 src/sage/geometry/polyhedron/ppl_lattice_polygon.py
**********************************************************************
File "src/sage/geometry/polyhedron/ppl_lattice_polygon.py", line 413, in sage.geometry.polyhedron.ppl_lattice_polygon.LatticePolygon_PPL_class.plot
Failed example:
    LatticePolytope_PPL([0], [1]).plot()
Expected:
    Graphics object consisting of 2 graphics primitives
Got:
    Graphics object consisting of 3 graphics primitives

comment:29 Changed 8 years ago by git

Commit: f07d43ce5c9fb71200c014e190e9b697c9c2c8616ec70bf13975c67af9fe909bc37d78fff789213b

Branch pushed to git repo; I updated commit sha1. New commits:

af227bdMerge branch 'u/chapoton/13890' into 6.5.rc3
6ec70bftrac #13890 correct failing doctest in lattice polytopes

comment:30 Changed 8 years ago by git

Commit: 6ec70bf13975c67af9fe909bc37d78fff789213b070976c3f380640876f96cf9fbb1bc603aa87d36

Branch pushed to git repo; I updated commit sha1. New commits:

070976ctrac #13890 correct failing doctest in words

comment:31 Changed 8 years ago by Frédéric Chapoton

Status: needs_workneeds_review

Should be good now.

comment:32 Changed 8 years ago by Volker Braun

Status: needs_reviewpositive_review

comment:33 Changed 8 years ago by Volker Braun

Branch: u/chapoton/13890070976c3f380640876f96cf9fbb1bc603aa87d36
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.