Ticket #8353 (closed enhancement: fixed)
Adding height() and width() functions to square grid paths
| Reported by: | abmasse | Owned by: | abmasse |
|---|---|---|---|
| Priority: | major | Milestone: | sage-4.3.4 |
| Component: | combinatorics | Keywords: | paths, height, width |
| Cc: | slabbe, sage-combinat | Work issues: | |
| Report Upstream: | N/A | Reviewers: | Sébastien Labbé |
| Authors: | Alexandre Blondin Massé | Merged in: | sage-4.3.4.alpha1 |
| Dependencies: | Stopgaps: |
Description (last modified by abmasse) (diff)
When dealing with 2d word paths, it is very useful to know their height and their width.
In particular, one can compute a bounding box for a better displaying. The aim of this small ticket is to add those two functionalities.
By the way, while testing it, I noticed a strange behavior:
sage: Paths = WordPaths('abcABC')
sage: p = Paths('acABC')
sage: points = list(p.points())
sage: points
[(0, 0), (1, 0), (1/2, 1/2*sqrt3), (-1/2, 1/2*sqrt3), (-1, 0), (-1/2, -1/2*sqrt3)]
sage: y_coords = map(lambda (_,y):y, points)
sage: y_coords
[0, 0, 1/2*sqrt3, 1/2*sqrt3, 0, -1/2*sqrt3]
sage: max(y_coords)
-1/2*sqrt3
Shouldn't 1/2*sqrt3 be the highest element ? This doesn't make sense.
Attachments
Change History
comment:6 Changed 3 years ago by slabbe
- Status changed from needs_review to needs_work
I would suggest that the value be converted to RR before taking the max or min. You could add a comment explaining the reason of the conversion and that this bug is trac at a ticket number that you give.
The description of this ticket is not really the good place to put your comment. You can put it in a comment of the ticket instead or on sage-devel. I asked the question on sage-devel : http://groups.google.com/group/sage-devel/browse_thread/thread/2557a48ad695b42e
comment:7 Changed 3 years ago by abmasse
- Owner changed from sage-combinat to abmasse
Thanks for your comments !
I found an almost-clean solution for the problem raised by the triangular grid paths. Instead of computing directly the height() and width() functions, I introduced the xmin(), xmax(), ymin() and ymax() functions we talked about. They are called by the two first functions. To solve the problem for the triangular grid paths, I just redefined them by coercing the x- and y-coordinates to real values, so that the max and min functions be well-defined.
I'll upload another patch in a few minutes. Needs review !
Changed 3 years ago by abmasse
-
attachment
trac_8353_square_grid_word_paths_height_width-abm.patch
added
New patch -- adds functions xmin(), xmax(), etc.
Changed 3 years ago by slabbe
-
attachment
trac_8353_review-sl.patch
added
Applies over the precedent patch
comment:9 Changed 3 years ago by slabbe
- Reviewers set to Sébastien Labbé
- Authors set to Alexandre Blondin Massé
All tests passed in sage/combinat/words. Documentation builds fine. The issue discussed above is fixed. I am giving a positive review to Alex's changes.
I added a small patch fixing some documentation. If Alexandre agrees with the changes I did, I allow him to change the status of the ticket to positive review.
comment:10 Changed 3 years ago by abmasse
- Status changed from needs_review to positive_review
I agree with Sébastien's changes. I retested it just to make sure, and I looked at the documentation, which is still fine. I'll set this ticket to "positive review".
comment:11 Changed 3 years ago by mhansen
- Status changed from positive_review to closed
- Resolution set to fixed
- Merged in set to sage-4.3.4.alpha1
