Hi Travis,
This all looks very good. Here's a few comments and suggestions:
- Added comparison methods to Partition_class
>
If you have he stomach for it, I think that it would also be good to name
the class Partition_class as Partition -- and make the current Partition
function its __classcall_private__ method.
> - Made Partitions_n inherit from Partitions
>
A more descriptive name for this class would be Partitions_size -- in which
case self.n should be self.size.
Right now there are about 8 tests which fail in partition.py. 5 of them are
> because I don't know how to get the old pickle information for the classes
> I changed, so Andrew any advice?
Apply my pickle documentation patch and read the new improved documentation!
A quicker solution is just to copy the __setstate__ method from the Tableau
class into the partition class (that is, into Partition_class), and then
adjust the documentation and - most importantly -- changing Tableaux() to
Partitions(). This will almost certainly fix most, if not all, of your
unpickling problems. The problem arises because partitions now inherits
from Element which means that it now has a sensible __setstate__ method.
Unfortunately, as a by-product, this also breaks the old pickles.
If you do change Partition_class to Partition, as I suggested above, then
you will also need to add the lines
from sage.structure.sage_object import register_unpickle_override
register_unpickle_override('sage.combinat.partition', 'Partition_class',
Partition)
to (the bottom of) partition.py. If any other class names have changed then
you will also need to add similar function calls for them.
Let me know if you have any problems.
Also there are a few questions I want to ask. Tableaux should also have
> options (French vs. English in particular), but should they be linked to
> the options for Partitions? Same question to PartitionTuples with
> Partitions. Does anybody disagree with or have any
> suggestions/recommendations on any of the above changes?
>
It would make sense to have the English/French convention shared between
partitions and tableaux --- and, possibly, their tupled friends although I
don't have strong feelings about the latter. On the other hand, you
probably would not want to do this for all options so if they some linked
this would confuse at least some people because they would not known which
options were in sync and which not.
Perhaps the rule should be that "conventions" are synchronised between
classes but "options" are not. (Please do not ask me to define these
terms!:)
Andrew
--
You received this message because you are subscribed to the Google Groups
"sage-combinat-devel" group.
To view this discussion on the web visit
https://groups.google.com/d/msg/sage-combinat-devel/-/Ra2ow_EcHOQJ.
To post to this group, send email to sage-combinat-devel@googlegroups.com.
To unsubscribe from this group, send email to
sage-combinat-devel+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/sage-combinat-devel?hl=en.
