Parallelogram Polyomino

[boussica]

Implementation of the Parallelogram Polyominoes

[git]

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

​f32d04e

Add bijections to Dyck tableaux

[git]

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

​a9a732d

Improve the parallelogram Polyominoes

[git]

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

​c256ca1

Improve Parallelogram polyominoes

[git]

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

​3130e69

Add some bijection to trees and improve the doc

[git]

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

​176197e

Add bij. OrderedTrees to parallelogramPolyominos

[git]

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

​773047a

Add Combinatorail map in ParallelogramPolyomino

[chapoton]

New commits:

​4ad6b34	Merge branch 'u/boussica/parallelogram_polyomino' of ssh://trac.sagemath.org:22/sage into 16110
​600c74a	trac #16110 minor code formatting and doc details

[git]

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

​ebd0bb4

trac #16110 some typos corrected

[git]

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

​b5e4192

Add tests for special cases in PP.

[git]

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

​e40248f

Improve Parallelogram Polyomino drawing.

[git]

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

​0a361e6

Pep8 !

[git]

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

​b7584cb

Pep

[chapoton]

I have made a standalone branch available as u/chapoton/16110

Of course, it still depends on set factories, but not in the branch.

[chapoton]

here is a branch that applies, your was not applying anymore.

Adrien, is this "needs review" ?

[chapoton]

Well, I allow myself to put this into needs review..

[vdelecroix]

Bonjour à tous,

Sage does not start with the branch applied.

--> 173 class ParallelogramPolyomino(ClonableList):
        global ParallelogramPolyomino = undefined
        global ClonableList = None
    174     r"""
    175     The class of Parallelogram Polyominoes
    176     """
    177     __metaclass__ = ClasscallMetaclass
...
TypeError: Error when calling the metaclass bases
    metaclass conflict: the metaclass of a derived class must be
    a (non-strict) subclass of the metaclasses of all its bases

Vincent

[git]

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

​7fd7fba	Merge branch 'u/chapoton/16110' into 6.9.b6
​af521d9	fixing metaclass

[chapoton]

New commits:

​7fd7fba	Merge branch 'u/chapoton/16110' into 6.9.b6
​af521d9	fixing metaclass

[boussica]

To solve some merge/rebase difficulties. I create a new branch with all the code present in the branch (e467677f92400e46cb27142e0623e693aa5db2ac).

I add the parallelogram polyominoe in the documentation.

[git]

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

​ea8b3d9

Improve documentation

[git]

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

​67c4693

Improve parallelogram polyomino bijection + doc.

[git]

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

​feca1f0	Improve documentation
​5ffad37	Some documentation

[git]

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

​96ddaae	Improve documentation
​3957b90	Remove get_node() from abstract tree
​9ac66b2	Merge branch 't/16204/add_some_methods_in_trees' into t/16110/parallelogram_polyomino_2
​f4ec302	Small bug

[git]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

​0eb710d	trac 16204 some more doc, plus code details
​79dc6e6	Add Parallelogram polyominoes
​f52b822	Improve documentation
​107a220	Improve parallelogram polyomino bijection + doc.
​23751dd	Improve documentation
​667fe14	Some documentation
​1ff1156	Improve documentation
​170a6c0	Small bug
​425e97c	Solve Documentation problems
​5a90eb9	Merge branch 'u/boussica/parallelogram_polyomino_2' of git://trac.sagemath.org/sage into t/16110/parallelogram_polyomino_2

[git]

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

​c7270a6

Add some documentation

[git]

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

​99e238f

Add Documentation and Tests for PP.

[git]

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

​4c4fd49

Now PP have the good size : the half-permiter

[git]

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

​70cda63

Add some documentations

[git]

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

​0b995ab

Improve documentation

[git]

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

​ae4513e

Documentation improvement

[chapoton]

Still need more doc, every function must be documented:

combinat/parallelogram_polyomino.py: 80.9% (72 of 89)

Some error in doc formatting:

+[dochtml] [combinat ] /home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage/combinat/parallelogram_polyomino.py:docstring of sage.combinat.parallelogram_polyomino.default_tikz_options:10: ERROR: Unexpected indentation.
+[dochtml] [combinat ] /home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage/combinat/parallelogram_polyomino.py:docstring of sage.combinat.parallelogram_polyomino.default_tikz_options:15: ERROR: Unexpected indentation.

The function "breadth_node_paths_generator" has been given another name in #16204. So doctests here do not pass.

[chapoton]

I have made a refreshed branch. There still remains

1) to have full and clean documentation

2) to adapt to the new standard of global options introduced by #18555

---

New commits:

​9cbd427	Merge branch 'u/boussica/parallelogram_polyomino_2' in 7.3.rc0
​192d35d	trac 16110 refresh the code (partially)

[git]

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

​da7931e

Merge branch 'develop' into t/16110/public/16110

[git]

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

​5862882

Update documentation and port for sage 7.5

[git]

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

​91fb165

Implementation of LocalOption

[git]

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

​4b27924

Solve error when compiling documentation

[git]

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

​faa4de2

Doc updates

[git]

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

​35edd8d

Updat documentation of Parallelogram polyominoes

[git]

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

​34e14b9	Add some documentation
​e06550f	Merge branch 'public/16110' of git://trac.sagemath.org/sage into t/16110/public/16110
​e0b331b	Merge + failed test

[chapoton]

you should not use xrange but range

you should use python3 syntax for print, namely print("stuff")

see patchbot report

[git]

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

​4f0788d

Added some documentation and tests

[git]

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

​8ad387e	Improve documentation
​b74afb7	Merge branch 'public/16110' of git://trac.sagemath.org/sage into t/16110/public/16110

[git]

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

​08dc790

Corrected some tests.

[git]

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

​8c7fd34

Added some documentations

[chapoton]

once again:

you should not use xrange but range

you should use python3 syntax for print, namely print("stuff")

see patchbot report

[git]

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

​ae33d76	Merge branch 'public/16110' in 7.5.b5
​8ba9ddd	trac 16110 cleanup of xrange, print, pep8

[git]

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

​89b1f3d	Merge branch 'develop' into t/16110/public/16110
​473cf1d	Improve documentation and rename some functions

[chapoton]

see patchbot report for many problems (doc does not build, among others)

[git]

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

​09a098b

trac 16110 fixing the docbuild issue

[git]

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

​5d3b55c

trac 16110 fixing metaclass for python3

[chapoton]

Not full-coverage:

+combinat/parallelogram_polyomino.py: 93.8% (90 of 96)

And also should be lazy-imported.

[git]

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

​a926e0a

trac 16110 lazy import

[boussica]

Thank you, i will work on that Ticket Thursday. We have, now, a working group : ​https://wiki.sagemath.org/thursdaysbdx.

I think, i will finish this ticket thursday.

[git]

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

​f327a36

trac 16110 adding some # indirect doctest

[git]

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

​effab84

Add some bibiography, complete all remaining TODOS

[slabbe]

Two small comments to start:

1. I would change to_parallelogram_polyomino_Boussicault_Socci to _to_parallelogram_polyomino_Boussicault_Socci since it is available through the method to_parallelogram_polyomino.

2. I suggest to support Python 3 syntax as most as possible for any new module to get into sage for example by adding the following line at the top of the module:

   # Support of Python 3
   from __future__ import division, absolute_import, print_function, unicode_literals
   

[slabbe]

Some more comments (just looking at the code, no test done yet):

3. _get_path_in_pair_of_tree_from_box similar methods do not have doctests.

4. (many occurrences) References format do not follow the conventions:

+        By default, we use the bijection from the following article :
+            Ref. Delest, M.-P. and Viennot, G.
+            "Algebraic Languages and Polyominoes Enumeration."
+            Theoret. Comput. Sci. 34, 169-206, 1984.
+            (the bijection is described in the previous article on page 179 
+            and 180. An example is given in Figure 6)

See ​this page of the developper's manual:

All bibliographical references should be stored in the master bibliography file,
 SAGE_ROOT/src/doc/en/reference/references/index.rst, in the format [...]

5. Out of curiosity, why does the class _polyomino_row: needs to be inside the class instead of simply in the module?

6. The class

class _drawing_tool:
    r"""
    Technical class to produce TIKZ drawing.

is of larger interest than just the parallelogram polyominoes (I often need such thing). But I think it is anormal to

sage: from sage.combinat.parallelogram_polyomino import _drawing_tool

I think you should rather create a new module:

sage: from sage.misc.tikz import DrawingTool

I have the idea of geting my module ​TikzPicture into sage in the future. The class DrawingTool and TikzPicture could go in the same module. I let you create that module first into Sage.

For example if you install my package with sage -pip install slabbe, you may do:

sage: L = [[0,0,0,1,1,0,1,0,0,1,1,1],[1,1,1,0,0,1,1,0,0,1,0,0]]
sage: pp = ParallelogramPolyomino(L)
sage: from slabbe import TikzPicture
sage: TikzPicture(latex(pp)).pdf()
'[...]/62669/tikz_mZXN3H.pdf'

[slabbe]

7. bounce and bounce_path methods have no INPUT block.

8. Use one line instead of three in the example of area, bounce and bounce_path methods (other occurrences?):

sage: pp = ParallelogramPolyomino(
....:     [[0, 1], [1, 0]]
....: )
sage: pp.area()
1

sage: pp = ParallelogramPolyomino(
....:     [[1], [1]]
....: )

9. bad REST formatting in method box_is_root (missing :: maybe)

10. In from_dyck_word, add `` around the string 'Delest-Viennot'

11. In from_dyck_word, it says the default is foo, but it is None.

12. In get_node_position_from_box, mention the default value:

nb_crossed_nodes – a list containg just one integer.

13. INPUT block is missing in is_k_directed.

14. In to_binary_tree and to_dyck_word and to_ordered_tree, documentation does not follow the convention:

!diff
-- ``bijection`` – None (default) The name of bijection to use for 
-  the convertion. The possible value are, ‘Aval-Boussicault’.
+- ``bijection`` -- string or ``None`` (default: ``None``) The name of bijection to use for 
+  the convertion. The possible value are ``None`` or ``'Aval-Boussicault'``.

15. The string starting with This global option contains all the data needed by the Parallelogram classes to draw, display in ASCII, compile in latex a parallelogram polyomino. should contain an EXAMPLE:: block. Similarly with the string starting with This is the default TIKZ options. This option is used to configurate element of a drawing to allow TIKZ code generation..

[slabbe]

16. coverage is still not 100%:

    $ sage -coverage src/sage/combinat/parallelogram_polyomino.py 
    ------------------------------------------------------------------------
    SCORE src/sage/combinat/parallelogram_polyomino.py: 93.8% (90 of 96)
    

[slabbe]

17. (this is a preference of myself, you may prefer to ignore it if you want). I prefer

this

sage: L = [[0,0,0,1,1,0,1,0,0,1,1,1],[1,1,1,0,0,1,1,0,0,1,0,0]]
sage: pp=ParallelogramPolyomino(L)

or

sage: A = [0,0,0,1,1,0,1,0,0,1,1,1]
sage: B = [1,1,1,0,0,1,1,0,0,1,0,0]
sage: pp=ParallelogramPolyomino([A, B])

to

sage: pp=ParallelogramPolyomino(
....:     [[0,0,0,1,1,0,1,0,0,1,1,1],[1,1,1,0,0,1,1,0,0,1,0,0]]
....: )

18. Also, the Python convention says to add space here:

    -sage: pp=ParallelogramPolyomino(L)
    +sage: pp = ParallelogramPolyomino(L)
    

[slabbe]

19. sage: ParallelogramPolyominoes? should mention in the INPUT and EXAMPLES blocks the case size=None.

[slabbe]

20. This is strange:

sage: P = ParallelogramPolyominoes()
sage: P.is_finite()
True

21. This does not work (but maybe it is okay to not work):

sage: P = ParallelogramPolyominoes()
sage: len(list(P.some_elements()))    # this works
100
sage: P.random_element()
...
TypeError: int() argument must be a string or a number, not 'PlusInfinity'

[slabbe]

Changing the status to needs work (oups, it was already!).

Other than that, I get

sage -t --long src/sage/combinat/parallelogram_polyomino.py
    [484 tests, 3.09 s]
----------------------------------------------------------------------
All tests passed!

Tell me when it needs a next round of review.

[boussica]

Replying to slabbe:

20. This is strange:

sage: P = ParallelogramPolyominoes()
sage: P.is_finite()
True

21. This does not work (but maybe it is okay to not work):

sage: P = ParallelogramPolyominoes()
sage: len(list(P.some_elements()))    # this works
100
sage: P.random_element()
...
TypeError: int() argument must be a string or a number, not 'PlusInfinity'

In think it is a bug of set_factories. This example show the problem :

sage: from sage.structure.set_factories import *
sage: from sage.structure.set_factories_example import *
sage: XYPairs()
AllPairs
sage: APS = XYPairs()
sage: APS.is_finite()
True

I will report that bug.

[git]

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

​4051fe7

Add tests to obtain 100% of coverage.

[boussica]

Replying to slabbe:

16. coverage is still not 100%:

    $ sage -coverage src/sage/combinat/parallelogram_polyomino.py 
    ------------------------------------------------------------------------
    SCORE src/sage/combinat/parallelogram_polyomino.py: 93.8% (90 of 96)
    

Now we have 100% of coverage.

In ParallelogramPolyominoesFactory?, I have commented the function add_constraint() that become nonImplemented, because i don't use them in the set_factory. I don't know if it is a good way to solve the problem, but I don't use that function, so, I don't know how to test it.

[chapoton]

This is not correct :

+            EXAMPLES::
+
+            sage: PP=ParallelogramPolyomino

because you must indent the test lines by 4 spaces.

This is not correct :

+        EXAMPLES::
+            sage: pp = ParallelogramPolyomino(

because you must have an empty line after EXAMPLES::

This is not a correct way to add a doctest :

         Return a default policy.
+
+        sage: ParallelogramPolyominoes._default_policy

because you have forgotten EXAMPLES::

You can just remove "add_constraints(self, cons, args_opts):", I think.

[git]

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

​0b1f3ab	Merge branch 'public/16110' in 8.1.rc2
​c71a863	trac 16110 fixing my own comments

[chapoton]

bot is now green, so that we can start discuss the code

[mantepse]

I would like to see this merged! Here are a few comments concerning docstrings:

• the error messages in check need spell- and semantic checking :-)

• the references should be moved to sage-develop/src/doc/en/reference/references/index.rst

• I do not understand the docstring in lines 819-821:

      @staticmethod
      def __classcall_private__(cls, *args, **opts):
          r"""
          Ensure that parallelogram polyominoes created by the enumerated sets
          and directly are the same and that they are instances of
          :class:`ParallelogramPolyomino`.
  

• docstrings of widths (line 1702) and heights (line 1843) should begin with imperative.

• concerning val (line 1976): this looks like a useful function, why not make it public? It's docstring should be Determine whether the cell at the given position is inside the parallelogram polyomino.

• docstring of _polyomino_row could be This is an internal class representing a single row of a parallelogram polyomino.

• diagramme in line 2434 should be diagram.

[git]

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

​1a0686d	Merge branch 'public/16110' in 8.2.b2
​6d528fc	trac 16110 some details

[mantepse]

Sébastien, can we merge this? You can set it to positive review on my behalf, in case you are happy with it.

[slabbe]

Well, not all of the 21 comments I gave have been fixed. And I think all of them do not take much long to fix.

[git]

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

​8f9fe27

trac #16110 moved refs to main ref file

[mantepse]

ping?

[slabbe]

Adrien, I suggest you come at ​https://wiki.sagemath.org/thursdaysbdx this following Thursday and we finish this:)

[mantepse]

ping?

[boussica]

Replying to mantepse:

ping?

pong, i will finish this task now. I have now some time to do this.

[mantepse]

wonderful!

[git]

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

​79b037c	Merge branch 't/16110/public/16110' into 8.4.b
​9cb98d8	Small correction in reference/references/index.rst

[mantepse]

Les p be the bounce path of the parallelogram
polyomino (:meth:`bounce_path`).

should be

Let `p` be the bounce path of the parallelogram
polyomino (:meth:`bounce_path`).

     def _repr_(self):
       r"""
        Return the string representation of the set of
        parallelogram polyominoes

should be

     def _repr_(self):
       r"""
        Return the string representation of the set of
        parallelogram polyominoes.

[boussica]

There is no particular reason. Just to highlight that this class is a small tool dedicated to ParallelogramPolyominoe?.

Replying to slabbe:

Some more comments (just looking at the code, no test done yet):

5. Out of curiosity, why does the class _polyomino_row: needs to be inside the class instead of simply in the module?

[boussica]

Merging All Drawing Tools coming from you tikz package and the drawing tools of ParallelogramPolyomino? is a big work :

• we need to list all the needed functionality
• we need to merge the ParallelogramPolyomino? codes and the code of your package
• we need to integrate the result on sage
• we need to make tests and documentation

I think its better to open a new ticket in order to close this one. We will treat that new ticket after this one.

I opened that new ticket : ticket #26508

Replying to slabbe:

6. The class

class _drawing_tool:
    r"""
    Technical class to produce TIKZ drawing.

is of larger interest than just the parallelogram polyominoes (I often need such thing). But I think it is anormal to

sage: from sage.combinat.parallelogram_polyomino import _drawing_tool

I think you should rather create a new module:

sage: from sage.misc.tikz import DrawingTool

I have the idea of geting my module ​TikzPicture into sage in the future. The class DrawingTool and TikzPicture could go in the same module. I let you create that module first into Sage.

For example if you install my package with sage -pip install slabbe, you may do:

sage: L = [[0,0,0,1,1,0,1,0,0,1,1,1],[1,1,1,0,0,1,1,0,0,1,0,0]]
sage: pp = ParallelogramPolyomino(L)
sage: from slabbe import TikzPicture
sage: TikzPicture(latex(pp)).pdf()
'[...]/62669/tikz_mZXN3H.pdf'

[git]

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

​2966230	Comment 68 - ticket 16110
​779cafc	Add Input and Change some code formating
​92c1be1	Format some code in the doctest
​0c70493	See Review ticket 16110 comment 70 .10 .11. 12
​047ee01	Review ticket 16100 comment 70 .13 .14. 15

[boussica]

I don't find what is the problem. Can you expand on that remark ?

Replying to slabbe:

9. bad REST formatting in method box_is_root (missing :: maybe)

[git]

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

​74c2291

pep8 is the life ! Review Comment 72 .17

[git]

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

​a3c195b	Solve problems raised by patchbot
​37b4dbb	Improve documentation of PPs

[git]

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

​e29cdbb	Merge branch 't/16110/public/16110' into 8.5.b2
​b56f871	Review ticket 16100 comment 73 .19

[boussica]

Review - Commment 71 .16 - coverage is done. Review - Comment 72 .18 - This is done by commit 74c2291

[git]

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

​f11ba03

Review ticket 16100 comment 85

[git]

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

​bce053f

Pep 8

[boussica]

All the corrections of comment 85 was done.

The function classcall_private needs to be implemented to work with SetFactory?. With that function, the parent method will work correctly.

Replying to mantepse:

I would like to see this merged! Here are a few comments concerning docstrings:

• the error messages in check need spell- and semantic checking :-)

• the references should be moved to sage-develop/src/doc/en/reference/references/index.rst

• I do not understand the docstring in lines 819-821:

      @staticmethod
      def __classcall_private__(cls, *args, **opts):
          r"""
          Ensure that parallelogram polyominoes created by the enumerated sets
          and directly are the same and that they are instances of
          :class:`ParallelogramPolyomino`.
  

• docstrings of widths (line 1702) and heights (line 1843) should begin with imperative.

• concerning val (line 1976): this looks like a useful function, why not make it public? It's docstring should be Determine whether the cell at the given position is inside the parallelogram polyomino.

• docstring of _polyomino_row could be This is an internal class representing a single row of a parallelogram polyomino.

• diagramme in line 2434 should be diagram.

---

New commits:

​f11ba03	Review ticket 16100 comment 85
​bce053f	Pep 8

[git]

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

​0cf531c

Improve documentation

[git]

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

​4b84af1

Merge branch 't/16110/public/16110' into 8.5.b3

[git]

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

​05aa2f0	Merge branch 'public/16110' of trac.sagemath.org:sage into 16110
​c079d3e	16110: minor corrections during review

[slabbe]

1. I just reviewed the code. Looks good to me. I did some trivial modifications while I was looking at it. I allowed myself to push a commit on top of the public branch.

2. Using Sage with Python 3, I get 3 failures (see below). So I change the status to needs work because of that:

sage -t --long src/sage/combinat/parallelogram_polyomino.py
**********************************************************************
File "src/sage/combinat/parallelogram_polyomino.py", line 238, in sage.combinat.parallelogram_polyomino.LocalOptions.__setitem__
Failed example:
    o["display"]="?"
Expected:
    Current value : diagram
    {'default': 'list', 'values':
    {'diagram': 'diagram representation',
    'list': 'list representation'}}
Got:
    Current value : diagram
    {'default': 'list', 'values': {'list': 'list representation', 'diagram': 'diagram representation'}}
**********************************************************************
File "src/sage/combinat/parallelogram_polyomino.py", line 304, in sage.combinat.parallelogram_polyomino.LocalOptions.__call__
Failed example:
    o(display="?")
Expected:
    Current value : diagram
    {'default': 'list', 'values':
    {'diagram': 'diagram representation',
    'list': 'list representation'}}
Got:
    Current value : diagram
    {'default': 'list', 'values': {'list': 'list representation', 'diagram': 'diagram representation'}}
**********************************************************************
File "src/sage/combinat/parallelogram_polyomino.py", line 1538, in sage.combinat.parallelogram_polyomino.ParallelogramPolyomino.get_options
Failed example:
    pp.get_options()
Expected:
    Current options for ParallelogramPolyominoes_size
      - display:            list
      - drawing_components: {'diagram': True, 'bounce_1': False,
    'tree': False, 'bounce_0': False}
      - latex:              drawing
      - tikz_options:       {'color_bounce_1': u'blue',
    'color_bounce_0': u'red', 'point_size': 3.5, 'line_size': 1,
    'color_line': u'black', 'color_point': u'black', 'scale': 1,
    'mirror': None, 'rotation': 0, 'translation': [0, 0]}
Got:
    Current options for ParallelogramPolyominoes_size
      - display:            list
      - drawing_components: {'diagram': True, 'tree': False, 'bounce_0': False, 'bounce_1': False}
      - latex:              drawing
      - tikz_options:       {'scale': 1, 'line_size': 1, 'point_size': 3.5, 'color_line': 'black', 'color_point': 'black', 'color_bounce_0': 'red', 'color_bounce_1': 'blue', 'translation': [0, 0], 'rotation': 0, 'mirror': None}
**********************************************************************
3 items had failures:
   1 of   7 in sage.combinat.parallelogram_polyomino.LocalOptions.__call__
   1 of  11 in sage.combinat.parallelogram_polyomino.LocalOptions.__setitem__
   1 of   3 in sage.combinat.parallelogram_polyomino.ParallelogramPolyomino.get_options
    [554 tests, 3 failures, 0.26 s]
----------------------------------------------------------------------
sage -t --long src/sage/combinat/parallelogram_polyomino.py  # 3 doctests failed
----------------------------------------------------------------------

3. The branch now contains more than 70 commits including lots of merge with the most recent develop branch of Sage. I would suggest to rebase the branch on top of the most recent develop version of sage so that the history tree do not get to dirty because of this ticket. You may try git rebase -i HEAD~3 to rebase the last 3 commits for instance (this is usually what I do). Of course, rebasing the branch will change the hash of all involved commits. So this is okay only if nobody have code depending on that branch.

[git]

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

​4e7c7b0	Merge branch 't/16110/public/16110' into 8.5.b6
​41c18d9	Trac #16110: Fix display order in __setitem__ and in ...

[vklein]

Replying to slabbe:

2. Using Sage with Python 3, I get 3 failures (see below). So I change the status to needs work because of that:

Python3 errors fixed

[slabbe]

I now get

sage -t --long src/sage/combinat/parallelogram_polyomino.py
    [554 tests, 0.28 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------

both in Python 2 and 3. Documentation builds fine. Positive review.

[embray]

This tickets were closed as fixed after the Sage 8.5 release.