Opened 6 years ago
Closed 4 years ago
#13890 closed defect (fixed)
small bug in point3d
Reported by:  tjolivet  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)  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 6 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:2 Changed 5 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:3 Changed 5 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:4 Changed 5 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:5 Changed 4 years ago by
 Branch set to u/chapoton/13890
 Commit set to 745550226729b06d349c4ba83f6ab457f464d1b8
 Keywords plot3d point3d added
 Status changed from new to needs_review
comment:6 Changed 4 years ago by
 Status changed from needs_review to needs_work
comment:7 Changed 4 years ago by
 Commit changed from 745550226729b06d349c4ba83f6ab457f464d1b8 to 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 4 years ago by
 Commit changed from 52dda634446582b8bed2c36f0ddfe383451d37e6 to e042380341e6ccd0f664a0226eb8f3f729b17319
Branch pushed to git repo; I updated commit sha1. New commits:
e042380  trac #13890 final touch : case of empty list

comment:9 Changed 4 years ago by
 Status changed from needs_work to needs_review
New commits:
e042380  trac #13890 final touch : case of empty list

comment:10 Changed 4 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 4 years ago by
 Status changed from needs_review to needs_work
comment:12 Changed 4 years ago by
 Commit changed from e042380341e6ccd0f664a0226eb8f3f729b17319 to 795d88f861c24593000707fe277b3c0c86b06d03
comment:13 Changed 4 years ago by
 Status changed from needs_work to 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 4 years ago by
Aha! I see what happened. Glad I tried that out. I'll take a look at this momentarily.
comment:15 Changed 4 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 4 years ago by
 Commit changed from 795d88f861c24593000707fe277b3c0c86b06d03 to 39fc0177cbe6ad52befd22715d78efd4e86dce1e
comment:17 Changed 4 years ago by
ping ?
comment:18 Changed 4 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 4 years ago by
 Status changed from needs_review to needs_info
comment:20 Changed 4 years ago by
 Commit changed from 39fc0177cbe6ad52befd22715d78efd4e86dce1e to f07d43ce5c9fb71200c014e190e9b697c9c2c861
comment:21 Changed 4 years ago by
 Status changed from needs_info to needs_review
comment:22 Changed 4 years ago by
So the empty iterator should be 2d, not 3d (or nothing)? Just checking if this is the "right" design decision.
comment:23 Changed 4 years ago by
Sorry, I do not understand if you still have a concern or not ?
comment:24 Changed 4 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 4 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 4 years ago by
 Reviewers set to KarlDieter Crisman
 Status changed from needs_review to 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 4 years ago by
 Milestone changed from sage6.4 to sage6.5
comment:28 Changed 4 years ago by
 Status changed from positive_review to 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 4 years ago by
 Commit changed from f07d43ce5c9fb71200c014e190e9b697c9c2c861 to 6ec70bf13975c67af9fe909bc37d78fff789213b
comment:30 Changed 4 years ago by
 Commit changed from 6ec70bf13975c67af9fe909bc37d78fff789213b to 070976c3f380640876f96cf9fbb1bc603aa87d36
Branch pushed to git repo; I updated commit sha1. New commits:
070976c  trac #13890 correct failing doctest in words

comment:31 Changed 4 years ago by
 Status changed from needs_work to needs_review
Should be good now.
comment:32 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:33 Changed 4 years ago by
 Branch changed from u/chapoton/13890 to 070976c3f380640876f96cf9fbb1bc603aa87d36
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
trac #13890 allow use of iterators in point3d