Opened 3 years ago

Closed 3 years ago

#28304 closed enhancement (fixed)

add type information to _repr_ of Macaulay2 elements

Reported by: Markus Wageringel Owned by:
Priority: major Milestone: sage-9.0
Component: interfaces: optional Keywords: macaulay2
Cc: Dima Pasechnik, gh-antonleykin Merged in:
Authors: Markus Wageringel Reviewers: Dima Pasechnik, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 2324c95 (Commits, GitHub, GitLab) Commit: 2324c95431269e4d7ab5f0c4653a70442c36d652
Dependencies: Stopgaps:

Status badges

Description

In Macaulay2, the AfterPrint is responsible for adding useful type information about computed results. Since this information generally cannot be obtained easily otherwise, it should be added to the _repr_ of Macaulay2 elements. Compare for example

sage: macaulay2.eval('kernel matrix {{1, 2}, {3, 6}}')
image | 2  |
      | -1 |

                          2
ZZ-module, submodule of ZZ

to

sage: macaulay2(matrix([[1, 2], [3, 6]])).kernel()
image | 2  |
      | -1 |
sage: _.cls()  # this is much less informative
Module

Change History (18)

comment:1 Changed 3 years ago by Markus Wageringel

With #28303, it would be easy to implement this as follows:

  • src/sage/interfaces/macaulay2.py

    a b class Macaulay2Element(ExtraTabCompletion, ExpectElement): 
    802802        """
    803803        from sage.typeset.ascii_art import empty_ascii_art
    804804        P = self.parent()
    805         return P.eval('print(wrap(%d,"-",net %s))'
    806                       % (empty_ascii_art._terminal_width(), self._name),
    807                       strip=False)
     805        # In M2, the wrapped output is indented by the width of the prompt,
     806        # which we strip in Sage. We hardcode the width of the prompt to
     807        # 14=len('o1000000001 = '), which is tested in the doctests by the
     808        # output getting wrapped at 80 characters.
     809        width = 14 + empty_ascii_art._terminal_width()
     810        return P.eval('printWidth=%d;%s' % (width, self._name), strip=True)
    808811
    809812    def external_string(self):
    810813        """

This ticket would affect the repr of many Macaulay2 elements and consequently lots of doctests would need to be updated. I think this change is justified for its usefulness, but I welcome other ideas. Making this output optional would also be a possibility (QQbar has optional print options, too, for instance).

comment:2 Changed 3 years ago by Markus Wageringel

Type: PLEASE CHANGEenhancement

comment:3 Changed 3 years ago by Markus Wageringel

Authors: Markus Wageringel
Branch: u/gh-mwageringel/28304
Cc: Dima Pasechnik gh-antonleykin added
Commit: fd3ab693e19ef0c70d67a4ac73870d4809c37664
Status: newneeds_review

We discussed this issue at the IMA a few weeks ago. Dima suggested that including type information in the repr by default is not pythonic, but that Macaulay2 elements should have a method to access this kind of information instead. I think it would still be useful to be able to optionally enable printing this info, as this is what Macaulay2 does by default.

Here is a branch implementing both. It adds a method after_print_text() to obtain this type info from within Sage. Moreover, it adds an option macaulay2.options.after_print='yes' ('no' by default) to append the AfterPrint text to the repr of Macaulay2 elements. This option can then be enabled in the init.sage file, for example.

sage: m = macaulay2(matrix([[1, 2], [3, 6]])); m
| 1 2 |
| 3 6 |
sage: m.after_print_text()
         2        2
Matrix ZZ  <--- ZZ
sage: macaulay2.options.after_print = 'yes'
sage: m
| 1 2 |
| 3 6 |

         2        2
Matrix ZZ  <--- ZZ

New commits:

e25e16b28304: add after_print_text() for Macaulay2 elements
fd3ab6928304: add global options to Macaulay2 to enable AfterPrint

comment:4 Changed 3 years ago by Markus Wageringel

Milestone: sage-8.9sage-9.0

comment:5 Changed 3 years ago by git

Commit: fd3ab693e19ef0c70d67a4ac73870d4809c37664bcbf9aafa6e6977be1866f6973ef8ecba8bb37e7

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

bcbf9aa28304: add global options to Macaulay2 to enable AfterPrint

comment:6 Changed 3 years ago by Dima Pasechnik

Reviewers: Dima Pasechnik
Status: needs_reviewneeds_work

in docstrings, the 1st line should be followed by an empty line. So

+    def after_print_text(self):
+        r"""
+        Obtain the type information about this Macaulay2 element that is
+        displayed using ``AfterPrint`` in the Macaulay2 interpreter.

should become something like

+    def after_print_text(self):
+        r"""
+        Macaulay2 type information for ``self``
+
+        Obtain the type information for ``self``, as
+        displayed by ``AfterPrint`` in Macaulay2 interpreter.

Also, please document options class.

comment:7 Changed 3 years ago by git

Commit: bcbf9aafa6e6977be1866f6973ef8ecba8bb37e73fc7b990ececd87325e0d56ada938ec2f886fe97

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

3fc7b9928304: improve Macaulay2 documentation

comment:8 Changed 3 years ago by Markus Wageringel

Thank you for the quick feedback. I have shortened the first sentence of the docstring, so that it fits on a single line, and moved the examples for the after_print option to the options class. The super class GlobalOptions actually generates the docstring if absent, but it is good to also have some examples there. (This is currently broken in HTML docs with Python 3 #28698.)

I have also moved the import of deprecated_function_alias to the top of the file, so that its docstring does not get added to the HTML documentation.

comment:9 Changed 3 years ago by Markus Wageringel

Status: needs_workneeds_review

comment:10 Changed 3 years ago by Travis Scrimshaw

For the after_print option, IMO more natural values would be True and False rather than strings.

comment:11 in reply to:  10 ; Changed 3 years ago by Markus Wageringel

Replying to tscrim:

For the after_print option, IMO more natural values would be True and False rather than strings.

I completely agree with this. The problem is that sage.structure.global_options only seems to work with options of type string, but booleans do not work. I have not looked much into it yet, so cannot say how difficult it is to change this, but will report back once I know more.

comment:12 in reply to:  11 ; Changed 3 years ago by Travis Scrimshaw

Replying to gh-mwageringel:

Replying to tscrim:

For the after_print option, IMO more natural values would be True and False rather than strings.

I completely agree with this. The problem is that sage.structure.global_options only seems to work with options of type string, but booleans do not work. I have not looked much into it yet, so cannot say how difficult it is to change this, but will report back once I know more.

That is wrong. See combinat/partition.py:

        diagram_str = dict(default="*",
                         description='The character used for the cells when printing Ferrers diagrams',
                         checker=lambda char: isinstance(char,str))

So use

        after_print = dict(default=True,
                           description='Print more stuff',
                           checker=lambda val: isinstance(val, bool))

comment:13 Changed 3 years ago by git

Commit: 3fc7b990ececd87325e0d56ada938ec2f886fe972324c95431269e4d7ab5f0c4653a70442c36d652

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

2324c9528304: use booleans for after_print option

comment:14 in reply to:  12 Changed 3 years ago by Markus Wageringel

Replying to tscrim:

So use

        after_print = dict(default=True,
                           description='Print more stuff',
                           checker=lambda val: isinstance(val, bool))

Awesome. Thanks for pointing this out. I have changed it.

comment:15 Changed 3 years ago by Travis Scrimshaw

Dima, are you going to finish the review here or do you want me to do it?

comment:16 Changed 3 years ago by Dima Pasechnik

Hi Travis, please go ahead with your review. I am a bit pressed for time now.

comment:17 Changed 3 years ago by Travis Scrimshaw

Reviewers: Dima PasechnikDima Pasechnik, Travis Scrimshaw
Status: needs_reviewpositive_review

Okay, LGTM.

comment:18 Changed 3 years ago by Volker Braun

Branch: u/gh-mwageringel/283042324c95431269e4d7ab5f0c4653a70442c36d652
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.