#26225 closed enhancement (fixed)

py3: change the repr of Finite families

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.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 15 months ago by chapoton

  • Branch set to u/chapoton/26225
  • Commit set to ab5fe56d11e3123459320fba657b52a5a1e4f984
  • Status changed from new to needs_review

New commits:

ab5fe56py3: change the repr of Finite family (pprint their dict)

comment:2 Changed 15 months ago by git

  • Commit changed from ab5fe56d11e3123459320fba657b52a5a1e4f984 to d6f75609daa0f637674dd99035344dd76f20a3e3

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

d6f7560fixing some doctests

comment:3 Changed 15 months ago by tscrim

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 15 months ago by git

  • Commit changed from d6f75609daa0f637674dd99035344dd76f20a3e3 to 5fd14505b4c8a23ef1edaa936cb1c096ec08fe00

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

5fd1450trac 26225 better _repr_ for families, using _keys if not None

comment:5 Changed 15 months ago by git

  • Commit changed from 5fd14505b4c8a23ef1edaa936cb1c096ec08fe00 to 8fd4354935dfba4de03ceefdbd3176d24636991f

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

8fd4354trac 26225 fixing doctests

comment:6 Changed 15 months ago by embray

  • 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):: 
    3535#*****************************************************************************
    3636import types
    3737from copy import copy
     38from pprint import pformat, saferepr
    3839
    3940from six import itervalues
    4041from six.moves import range
    class FiniteFamily(AbstractFamily): 
    659660            sage: from sage.sets.family import FiniteFamily
    660661            sage: FiniteFamily({3: 'a'}) # indirect doctest
    661662            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'}
    662669        """
    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 15 months ago by tscrim

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 15 months ago by embray

Ok. Other than my above code style suggestion this is better then.

comment:9 Changed 15 months ago by git

  • Commit changed from 8fd4354935dfba4de03ceefdbd3176d24636991f to 10a2126b4661c7f0adf1da6c0b1aae5a31a2a7df

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

10a2126coding style

comment:10 Changed 15 months ago by chapoton

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 15 months ago by tscrim

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 15 months ago by git

  • Commit changed from 10a2126b4661c7f0adf1da6c0b1aae5a31a2a7df to 239f345c775496b4f09bb991c630322a51073502

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

239f345py3: change the repr of Finite family (pprint their dict)

comment:13 Changed 15 months ago by chapoton

thanks, done. I am now launching my patchbots.

comment:14 Changed 15 months ago by chapoton

ok, both bots are morally green

comment:15 Changed 15 months ago by tscrim

  • Reviewers set to Travis Scrimshaw

Can you check that this change does not affect #26078? If it does, could you make #26078 a dependency?

Otherwise LGTM (I don't know of any other tickets that would otherwise have a conflict with this change).

comment:16 Changed 15 months ago by chapoton

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 15 months ago by tscrim

  • Status changed from needs_review to positive_review

Ah, right, they are all instances of TrivialFamily. Thanks.

comment:18 Changed 15 months ago by vbraun

  • Branch changed from u/chapoton/26225 to 239f345c775496b4f09bb991c630322a51073502
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.