Changes between Initial Version and Version 2 of Ticket #15425


Ignore:
Timestamp:
05/04/14 23:15:37 (6 years ago)
Author:
nthiery
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #15425

    • Property Milestone changed from sage-6.1 to sage-6.2
  • Ticket #15425 – Description

    initial v2  
    1515  iterator
    1616
    17 {{{cartesian_product}}} is meant to subdue {{{CartesianProduct}}}. The
    18 missing features at this point are:
     17To be done:
    1918
    20 - Accepting any iterable as input. This probably requires turning them
    21   into parents (e.g. with FiniteEnumeratedSet). The overhead is
    22   probably negligible in most cases, but one would need to double
    23   check that we don't have spots where CartesianProduct is used
    24   intensively for very small calculations.
     19#.  Make {{{CartesianProduct}}} an alias for {{{cartesian_product}}},
     20    and possibly deprecated it.
    2521
    26   #14224 which can be closed once this is fixed.
     22    The missing features at this point are:
    2723
    28 - Some features of CartesianProduct still need to be lifted to
    29   Sets.Finite.CartesianProducts or EnumeratedSets.CartesianProducts.
    30   For example, cardinality is currently calculated from the iterator (gasp):
    31   {{{
    32       sage: F = Permutations(10)
    33       sage: C = cartesian_product([F,F])
    34       sage: C.cardinality()
    35       *hangs forever*
    36   }}}
     24    - Accepting any iterable as input. This probably requires turning them
     25      into parents (e.g. with FiniteEnumeratedSet). The overhead is
     26      probably negligible in most cases, but one would need to double
     27      check that we don't have spots where CartesianProduct is used
     28      intensively for very small calculations.
    3729
    38 Once this is done, we can make CartesianProduct an alias for
    39 cartesian_product, and maybe deprecate it in the long run.
     30      #14224 can be closed once this is fixed.
    4031
    41 My 2 cents for cartesian_product_iterator: it should be removed from
    42 the global name space, and deprecated altogether if after checking it
    43 turns out to be really just a duplicated of itertools.product.
     32    - Some features of CartesianProduct still need to be lifted to
     33      Sets.Finite.CartesianProducts or EnumeratedSets.CartesianProducts.
     34      For example, cardinality is currently calculated from the iterator (gasp):
     35      {{{
     36          sage: F = Permutations(10)
     37          sage: C = cartesian_product([F,F])
     38          sage: C.cardinality()
     39          *hangs forever*
     40      }}}
    4441
    45 Another bug in cartesian_product (reported by Vincent Delecroix [1]):
    46 {{{
    47 sage: C = cartesian_product([ZZ,ZZ])
    48 ...
    49 AttributeError: type object 'sage.rings.integer_ring.IntegerRing_class' has no attribute 'CartesianProduct'
    50 }}}
     42      Done by #16269/#10963 for cardinality and `__iter__`. Needs
     43      double checking for the infinite cases.
    5144
    52 This is a regression that is caused by a small change introduced by
    53 #12959 in Sets.ParentMethods.CartesianProduct:
    54 {{{
    55             return parents[0].__class__ -> return parents[0].__class__
    56 }}}
     45#.  Remove cartesian_product_iterator from the global name space, and
     46    deprecate it altogether if, after checking, it turns out to be
     47    really just a duplicated of itertools.product.
    5748
    58 I (Nicolas) take a double blame for it: I was reviewer of this ticket
    59 and did not notice this chunk (or don't remember why it was
    60 introduced), and I had not written an appropriate test in the first
    61 place. So this needs to be fixed too.
     49#.  Fix bug in cartesian_product (reported by Vincent Delecroix [1]):
     50    {{{
     51    sage: C = cartesian_product([ZZ,ZZ])
     52    ...
     53    AttributeError: type object 'sage.rings.integer_ring.IntegerRing_class' has no attribute 'CartesianProduct'
     54    }}}
    6255
    63 Yet another bug reported by Nathann Cohen [2]: when converting a list
    64 to an element of a cartesian product:
    65 {{{
    66 sage: Z3 = IntegerModRing(3)
    67 sage: C = cartesian_product([Z3,Z3])
    68 sage: C([Z3(2),Z3(2)])^2
    69 (1, 1)
    70 sage: C([2,2])^2   # Ooops
    71 (4, 4)
    72 }}}
     56    This is a regression that is caused by a small change introduced by
     57    #12959 in Sets.ParentMethods.CartesianProduct:
     58    {{{
     59                return parents[0].__class__ -> return parents[0].__class__
     60    }}}
    7361
    74 The fix would be convert the operands of the list into the respective
    75 parents in
    76 {{{sage.sets.cartesian_product.CartesianProduct._element_constructor}}}.
     62    I (Nicolas) take a double blame for it: I was reviewer of this ticket
     63    and did not notice this chunk (or don't remember why it was
     64    introduced), and I had not written an appropriate test in the first
     65    place. So this needs to be fixed too.
    7766
    78 Many features could be further added, like for example making the
    79 cartesian product of an additive magma into an additive magma and so
    80 on. This can go in this ticket or in later tickets.
     67#.  #16269: Fix bug reported by Nathann Cohen [2]: when converting a
     68    list to an element of a cartesian product:
     69    {{{
     70    sage: Z3 = IntegerModRing(3)
     71    sage: C = cartesian_product([Z3,Z3])
     72    sage: C([Z3(2),Z3(2)])^2
     73    (1, 1)
     74    sage: C([2,2])^2   # Ooops
     75    (4, 4)
     76    }}}
     77
     78    The fix would be convert the operands of the list into the respective
     79    parents in
     80    {{{sage.sets.cartesian_product.CartesianProduct._element_constructor}}}.
     81
     82#.  Many features could be further added, like for example making the
     83    cartesian product of an additive magma into an additive magma and
     84    so on. A good step was done with #16269. Another step needs to be
     85    done after #10963 to ventilate the features in the appropriate
     86    axiom categories.
    8187
    8288[1] https://groups.google.com/forum/#!topic/sage-combinat-devel/8Aw63kro_0M