Opened 2 years ago
Closed 2 years ago
#26225 closed enhancement (fixed)
py3: change the repr of Finite families
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage8.4 
Component:  python3  Keywords:  
Cc:  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  239f345 (Commits)  Commit:  239f345c775496b4f09bb991c630322a51073502 
Dependencies:  Stopgaps: 
Description
by pprinting their dict (sort by key)
Change History (18)
comment:1 Changed 2 years ago by
 Branch set to u/chapoton/26225
 Commit set to ab5fe56d11e3123459320fba657b52a5a1e4f984
 Status changed from new to needs_review
comment:2 Changed 2 years ago by
 Commit changed from ab5fe56d11e3123459320fba657b52a5a1e4f984 to d6f75609daa0f637674dd99035344dd76f20a3e3
Branch pushed to git repo; I updated commit sha1. New commits:
d6f7560  fixing some doctests

comment:3 Changed 2 years ago by
General +1 to this change, but see comment 4 of #24438. In particular, having a _keys
means the iteration order for FiniteFamily
is fixed, and I think the repr should reflect that.
comment:4 Changed 2 years ago by
 Commit changed from d6f75609daa0f637674dd99035344dd76f20a3e3 to 5fd14505b4c8a23ef1edaa936cb1c096ec08fe00
Branch pushed to git repo; I updated commit sha1. New commits:
5fd1450  trac 26225 better _repr_ for families, using _keys if not None

comment:5 Changed 2 years ago by
 Commit changed from 5fd14505b4c8a23ef1edaa936cb1c096ec08fe00 to 8fd4354935dfba4de03ceefdbd3176d24636991f
Branch pushed to git repo; I updated commit sha1. New commits:
8fd4354  trac 26225 fixing doctests

comment:6 Changed 2 years ago by

src/sage/sets/family.py
diff git a/src/sage/sets/family.py b/src/sage/sets/family.py index db94e7e..a780518 100644
a b Check :trac:`12482` (shall be run in a fresh session):: 35 35 #***************************************************************************** 36 36 import types 37 37 from copy import copy 38 from pprint import pformat, saferepr 38 39 39 40 from six import itervalues 40 41 from six.moves import range … … class FiniteFamily(AbstractFamily): 659 660 sage: from sage.sets.family import FiniteFamily 660 661 sage: FiniteFamily({3: 'a'}) # indirect doctest 661 662 Finite family {3: 'a'} 663 664 sage: FiniteFamily({3: 'a', 4: 'b'}) # indirect doctest 665 Finite family {3: 'a', 4: 'b'} 666 667 sage: FiniteFamily({3: 'a', 4: 'b'}, keys=[4,3]) # indirect doctest 668 Finite family {4: 'b', 3: 'a'} 662 669 """ 663 return "Finite family %s"%self._dictionary 670 if self._keys is None: 671 d = ' '.join(pformat(self._dictionary).splitlines()) 672 return "Finite family %s" % d 673 674 d = ', '.join('{}: {}'.format(saferepr(key), 675 saferepr(self._dictionary[key])) 676 for key in self._keys) 677 return 'Finite family {{{}}}'.format(d)
You could avoid some duplication (and inconsistency in string formatting methods) by writing this like:
if self._keys: d = ', '.join('{}: {}'.format(saferepr(key), saferepr(self._dictionary[key])) for key in self._keys) else: d = ' '.join(pformat(self._dictionary)[1:1].splitlines()) return 'Finite family {{{}}}'.format(d)
or similar.
Travis, does this address the problems with #24438? I haven't checked carefully. Doesn't this version mean that elements not in the keys
argument won't be displayed at all? Is that desirable?
comment:7 Changed 2 years ago by
My understanding is this ticket would be another way to fix #24438, but a nuance may be lost on me. It should be that keys
should match the keys of the dict object used in the construction (well, more generally, keys
is treated as the input to a function  equivalently, a sequence indexed by keys
 so things not in keys
should not be displayed).
comment:8 Changed 2 years ago by
Ok. Other than my above code style suggestion this is better then.
comment:9 Changed 2 years ago by
 Commit changed from 8fd4354935dfba4de03ceefdbd3176d24636991f to 10a2126b4661c7f0adf1da6c0b1aae5a31a2a7df
Branch pushed to git repo; I updated commit sha1. New commits:
10a2126  coding style

comment:10 Changed 2 years ago by
There is an annoying doctest failure in
src/sage/algebras/lie_algebras/classical_lie_algebra.py
which is not clear to me. The order seems to change from one machine to another..
comment:11 Changed 2 years ago by
So it has to do with the fact that roots and coroots are incomparable, hence sorted
is random but the underlying dict is small enough that it doesn't visibly suffer from this. What we should do is just add a fixed set of keys. So for str_keys
, it would be
keys = (['e{}'.format(i) for i in index_set] + ['f{}'.format(i) for i in index_set] + ['h{}'.format(i) for i in index_set])
and the other case would be
keys = ([alpha[i] for i in index_set] + [alpha[i] for i in index_set] + [alphacheck[i] for i in index_set])
and then
Family(keys, ret.__getitem__)
comment:12 Changed 2 years ago by
 Commit changed from 10a2126b4661c7f0adf1da6c0b1aae5a31a2a7df to 239f345c775496b4f09bb991c630322a51073502
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
239f345  py3: change the repr of Finite family (pprint their dict)

comment:13 Changed 2 years ago by
thanks, done. I am now launching my patchbots.
comment:14 Changed 2 years ago by
ok, both bots are morally green
comment:15 Changed 2 years ago by
 Reviewers set to Travis Scrimshaw
comment:16 Changed 2 years ago by
It should not interfere, as here we only change the output of "Finite family", not the output of "Family". And there is no "Finite Family" in #26078.
comment:17 Changed 2 years ago by
 Status changed from needs_review to positive_review
Ah, right, they are all instances of TrivialFamily
. Thanks.
comment:18 Changed 2 years ago by
 Branch changed from u/chapoton/26225 to 239f345c775496b4f09bb991c630322a51073502
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
py3: change the repr of Finite family (pprint their dict)