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:  sage6.5 
Component:  graphics  Keywords:  plot3d, point3d 
Cc:  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  KarlDieter Crisman 
Report Upstream:  N/A  Work issues:  
Branch:  070976c (Commits, GitHub, GitLab)  Commit:  070976c3f380640876f96cf9fbb1bc603aa87d36 
Dependencies:  Stopgaps: 
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/sage5.5/local/lib/python2.7/sitepackages/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/sage5.5/local/lib/python2.7/sitepackages/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
Milestone:  sage5.11 → sage5.12 

comment:2 Changed 9 years ago by
Milestone:  sage6.1 → sage6.2 

comment:3 Changed 8 years ago by
Milestone:  sage6.2 → sage6.3 

comment:4 Changed 8 years ago by
Milestone:  sage6.3 → sage6.4 

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

Branch:  → u/chapoton/13890 
Commit:  → 745550226729b06d349c4ba83f6ab457f464d1b8 
Keywords:  plot3d point3d added 
Status:  new → needs_review 
comment:6 Changed 8 years ago by
Status:  needs_review → needs_work 

comment:7 Changed 8 years ago by
Commit:  745550226729b06d349c4ba83f6ab457f464d1b8 → 52dda634446582b8bed2c36f0ddfe383451d37e6 

Branch pushed to git repo; I updated commit sha1. New commits:
52dda63  trac #13890 suggestion to use point3d instead for iterator input

comment:8 Changed 8 years ago by
Commit:  52dda634446582b8bed2c36f0ddfe383451d37e6 → e042380341e6ccd0f664a0226eb8f3f729b17319 

Branch pushed to git repo; I updated commit sha1. New commits:
e042380  trac #13890 final touch : case of empty list

comment:9 Changed 8 years ago by
Status:  needs_work → needs_review 

New commits:
e042380  trac #13890 final touch : case of empty list

comment:10 Changed 8 years ago by
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
Status:  needs_review → needs_work 

comment:12 Changed 8 years ago by
Commit:  e042380341e6ccd0f664a0226eb8f3f729b17319 → 795d88f861c24593000707fe277b3c0c86b06d03 

comment:13 Changed 8 years ago by
Status:  needs_work → needs_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
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
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
Commit:  795d88f861c24593000707fe277b3c0c86b06d03 → 39fc0177cbe6ad52befd22715d78efd4e86dce1e 

comment:18 Changed 8 years ago by
Sorry, other business (Sagewise 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
Status:  needs_review → needs_info 

comment:20 Changed 8 years ago by
Commit:  39fc0177cbe6ad52befd22715d78efd4e86dce1e → f07d43ce5c9fb71200c014e190e9b697c9c2c861 

comment:21 Changed 8 years ago by
Status:  needs_info → needs_review 

comment:22 Changed 8 years ago by
So the empty iterator should be 2d, not 3d (or nothing)? Just checking if this is the "right" design decision.
comment:24 Changed 8 years ago by
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
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
Reviewers:  → KarlDieter Crisman 

Status:  needs_review → positive_review 
Yes, upon further thought you are right, that was a preexisting thing. Thanks! Sorry this small thing ended up going crazy long.
comment:27 Changed 8 years ago by
Milestone:  sage6.4 → sage6.5 

comment:28 Changed 8 years ago by
Status:  positive_review → needs_work 

Some doctests fail, e.g.
sage t long warnlong 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
Commit:  f07d43ce5c9fb71200c014e190e9b697c9c2c861 → 6ec70bf13975c67af9fe909bc37d78fff789213b 

comment:30 Changed 8 years ago by
Commit:  6ec70bf13975c67af9fe909bc37d78fff789213b → 070976c3f380640876f96cf9fbb1bc603aa87d36 

Branch pushed to git repo; I updated commit sha1. New commits:
070976c  trac #13890 correct failing doctest in words

comment:32 Changed 8 years ago by
Status:  needs_review → positive_review 

comment:33 Changed 8 years ago by
Branch:  u/chapoton/13890 → 070976c3f380640876f96cf9fbb1bc603aa87d36 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
trac #13890 allow use of iterators in point3d