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: 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) 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/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 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:2 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:3 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:4 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:5 Changed 5 years ago by chapoton

  • Authors set to Frédéric Chapoton
  • Branch set to u/chapoton/13890
  • Commit set to 745550226729b06d349c4ba83f6ab457f464d1b8
  • Keywords plot3d point3d added
  • Status changed from new to needs_review

New commits:

7455502trac #13890 allow use of iterators in point3d

comment:6 Changed 5 years ago by chapoton

  • Status changed from needs_review to needs_work

comment:7 Changed 5 years ago by git

  • Commit changed from 745550226729b06d349c4ba83f6ab457f464d1b8 to 52dda634446582b8bed2c36f0ddfe383451d37e6

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

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

comment:8 Changed 5 years ago by git

  • Commit changed from 52dda634446582b8bed2c36f0ddfe383451d37e6 to e042380341e6ccd0f664a0226eb8f3f729b17319

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

e042380trac #13890 final touch : case of empty list

comment:9 Changed 5 years ago by chapoton

  • Status changed from needs_work to needs_review

New commits:

e042380trac #13890 final touch : case of empty list

comment:10 Changed 5 years ago by kcrisman

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 5 years ago by kcrisman

  • Status changed from needs_review to needs_work

comment:12 Changed 5 years ago by git

  • Commit changed from e042380341e6ccd0f664a0226eb8f3f729b17319 to 795d88f861c24593000707fe277b3c0c86b06d03

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 5 years ago by chapoton

  • 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 5 years ago by kcrisman

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

comment:15 Changed 5 years ago by kcrisman

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 5 years ago by git

  • Commit changed from 795d88f861c24593000707fe277b3c0c86b06d03 to 39fc0177cbe6ad52befd22715d78efd4e86dce1e

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 4 years ago by chapoton

ping ?

comment:18 Changed 4 years ago by kcrisman

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 4 years ago by mmezzarobba

  • Status changed from needs_review to needs_info

comment:20 Changed 4 years ago by git

  • Commit changed from 39fc0177cbe6ad52befd22715d78efd4e86dce1e to f07d43ce5c9fb71200c014e190e9b697c9c2c861

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 4 years ago by chapoton

  • Status changed from needs_info to needs_review

comment:22 Changed 4 years ago by kcrisman

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 chapoton

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

comment:24 Changed 4 years ago by kcrisman

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 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 4 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman
  • Status changed from needs_review to positive_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 4 years ago by kcrisman

  • Milestone changed from sage-6.4 to sage-6.5

comment:28 Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_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 4 years ago by git

  • Commit changed from f07d43ce5c9fb71200c014e190e9b697c9c2c861 to 6ec70bf13975c67af9fe909bc37d78fff789213b

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 4 years ago by git

  • Commit changed from 6ec70bf13975c67af9fe909bc37d78fff789213b to 070976c3f380640876f96cf9fbb1bc603aa87d36

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

070976ctrac #13890 correct failing doctest in words

comment:31 Changed 4 years ago by chapoton

  • Status changed from needs_work to needs_review

Should be good now.

comment:32 Changed 4 years ago by vbraun

  • Status changed from needs_review to positive_review

comment:33 Changed 4 years ago by vbraun

  • Branch changed from u/chapoton/13890 to 070976c3f380640876f96cf9fbb1bc603aa87d36
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.