Make Sequence pretty-printed in IPython#39027
Conversation
6f748f1 to
309f356
Compare
|
Documentation preview for this PR (built with commit 2d5ce6d; changes) is ready! 🎉 |
|
In general, I like the new output more. Especially: Since its also aligns well with standard IPython behavior, I would say go ahead and fix the tests so we can merge it. |
|
I realize this is rather problematic: the current behavior ignores if len(self) < 20:
return Sequence_generic._repr_(self)
else:
return "Polynomial Sequence with %d Polynomials in %d Variables" % (len(self),self.nvariables())I propose we add back Also avoid confusion like #26792 (comment) |
1c240dd to
76bc991
Compare
|
I just went through the whole diff and notice a handful of |
sagemathgh-39027: Make Sequence pretty-printed in IPython Related to sagemath#36801 , but this does not resolve that one. Instead: * modify the function to respect `_repr_pretty_` just like IPython * implement `_repr_pretty_` for `Sequence` and `PolynomialSequence` * Add notes that `_repr_` and `cr=` and `cr_str=` of `Sequence` are mostly unused * Add `# need sage.groups` (automatic fix by sage-fixdoctests) When sagemath#36801 is implemented part of this would be redundant (because `Sequence` derives from `list`) ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. - [x] fix doc tests - [x] explain meaning of `cr=True` in `Sequence` constructor ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39027 Reported by: user202729 Reviewer(s): Tobias Diez
sagemathgh-39420: Replace reproducible_repr() with usage of displayhook * It is not very well-known that doctest has magic features to ignore the ordering of `dict` and `set` (see for example sagemath#39413 (comment) ), I try to make it more discoverable. * Since the displayhook is in place (since 68b10e1), the `reproducible_repr` (added in e68d6eb) is mostly redundant. * Implement `_repr_pretty_` for `KeyConvertingDict` to make doctest deterministic. (based on the framework of sagemath#39027) There's a slight difference between `reproducible_repr` ordering (which always order by string representation) and this function (which tries first to order by element values, then by string representation), and the ordering of elements is reversed for multivariate polynomial ring (in `R.<x,y,z> = QQ[]`, `z < y < x`). However, it should still be deterministic. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39420 Reported by: user202729 Reviewer(s): Tobias Diez, user202729
sagemathgh-39420: Replace reproducible_repr() with usage of displayhook * It is not very well-known that doctest has magic features to ignore the ordering of `dict` and `set` (see for example sagemath#39413 (comment) ), I try to make it more discoverable. * Since the displayhook is in place (since 68b10e1), the `reproducible_repr` (added in e68d6eb) is mostly redundant. * Implement `_repr_pretty_` for `KeyConvertingDict` to make doctest deterministic. (based on the framework of sagemath#39027) There's a slight difference between `reproducible_repr` ordering (which always order by string representation) and this function (which tries first to order by element values, then by string representation), and the ordering of elements is reversed for multivariate polynomial ring (in `R.<x,y,z> = QQ[]`, `z < y < x`). However, it should still be deterministic. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39420 Reported by: user202729 Reviewer(s): Tobias Diez, user202729
sagemathgh-39420: Replace reproducible_repr() with usage of displayhook * It is not very well-known that doctest has magic features to ignore the ordering of `dict` and `set` (see for example sagemath#39413 (comment) ), I try to make it more discoverable. * Since the displayhook is in place (since 68b10e1), the `reproducible_repr` (added in e68d6eb) is mostly redundant. * Implement `_repr_pretty_` for `KeyConvertingDict` to make doctest deterministic. (based on the framework of sagemath#39027) There's a slight difference between `reproducible_repr` ordering (which always order by string representation) and this function (which tries first to order by element values, then by string representation), and the ordering of elements is reversed for multivariate polynomial ring (in `R.<x,y,z> = QQ[]`, `z < y < x`). However, it should still be deterministic. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39420 Reported by: user202729 Reviewer(s): Tobias Diez, user202729
Related to #36801 , but this does not resolve that one. Instead:
_repr_pretty_just like IPython_repr_pretty_forSequenceandPolynomialSequence_repr_andcr=andcr_str=ofSequenceare mostly unused# need sage.groups(automatic fix by sage-fixdoctests)When #36801 is implemented part of this would be redundant (because
Sequencederives fromlist)📝 Checklist
cr=TrueinSequenceconstructor⌛ Dependencies