Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#27848 closed enhancement (fixed)

Deprecate to_sage() and structure_sheaf() in Macaulay2 interface

Reported by: gh-mwageringel Owned by:
Priority: major Milestone: sage-8.9
Component: interfaces Keywords: macaulay2, deprecation
Cc: Merged in:
Authors: Markus Wageringel Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: e0ead46 (Commits, GitHub, GitLab) Commit: e0ead46643b671d0158424e98c7e77120f9b7ff8
Dependencies: Stopgaps:

Status badges

Description

For consistency with other interfaces, the method to_sage() for converting Macaulay2Elements to Sage should be deprecated and replaced by sage(). More precisely, the method _sage_() inherited from a super class should be implemented.

Similarly, the Sage method structure_sheaf() should be deprecated in favor of the Macaulay2 function sheaf() which provides the same functionality. Since structure_sheaf() is implemented for any Macaulay2Element, even for elements that do not support it, it unnecessarily pollutes the tab completion menu. In contrast, sheaf() only appears in the completion menu if an element supports it.

Change History (8)

comment:1 Changed 2 years ago by gh-mwageringel

  • Authors set to Markus Wageringel
  • Branch set to u/gh-mwageringel/macaulay2_deprecations
  • Commit set to 9f0b05c1dd6f28cc28b6baadad52153b077b565b
  • Status changed from new to needs_review

I have implemented the deprecations and also fixed all the failing doctests involving Macaulay2, which I tested using Macaulay2 1.11 and 1.13 on OS X.

Since the Macaulay2 description of matrices changed in 1.13, I also added code to ensure these matrices have the correct string representation within Sage (see the commit message for details).

To run the tests on all files that have optional Macaulay2 doctests, you can use:

sage -t --long --optional=sage,macaulay2 $(grep -i "optional.*macaulay2" -r src/sage -l | paste -sd " " -)

New commits:

8bd9f5dFixed further Macaulay2-related doctests
406beb2fix ascii art for matrices in Macaulay2 1.13+
50bc660deprecate Macaulay2Element.to_sage
9f0b05cdeprecate Macaulay2Element.structure_sheaf

comment:2 Changed 2 years ago by chapoton

With python3-sage, and M2 (version 1.14) installed and working, and this ticket's branch, I get

sage -t --long src/sage/matrix/matrix1.pyx  # 2 doctests failed
sage -t --long src/sage/interfaces/macaulay2.py  # 2 doctests failed

Some of these failures may be there without the branch here.

Last edited 2 years ago by chapoton (previous) (diff)

comment:3 Changed 2 years ago by chapoton

The matrix failure should be an easy fix: please replace

entries = map(list, self)

by

entries = [list(c) for c in self]

in the macaulay method in the file matrix1.pyx.

comment:4 Changed 2 years ago by chapoton

The other failure is a "bytes versus string" problem.

EDIT:

This shoud be fixed by replacing

return s

by return bytes_to_str(s) in the _sage_src_ method of the M2 interface.

Last edited 2 years ago by chapoton (previous) (diff)

comment:5 Changed 2 years ago by chapoton

  • Branch changed from u/gh-mwageringel/macaulay2_deprecations to public/ticket/27848
  • Commit changed from 9f0b05c1dd6f28cc28b6baadad52153b077b565b to e0ead46643b671d0158424e98c7e77120f9b7ff8
  • Reviewers set to Frédéric Chapoton

I have made my suggested changes. If you agree with them, you can set this ticket to positive review.


New commits:

a4036cdFixed further Macaulay2-related doctests
c0321d3fix ascii art for matrices in Macaulay2 1.13+
7fe0483deprecate Macaulay2Element.to_sage
68b5715deprecate Macaulay2Element.structure_sheaf
e0ead46py3 fixes

comment:6 Changed 2 years ago by gh-mwageringel

  • Status changed from needs_review to positive_review

Ok, thank you.

comment:7 Changed 2 years ago by vbraun

  • Branch changed from public/ticket/27848 to e0ead46643b671d0158424e98c7e77120f9b7ff8
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:8 Changed 2 years ago by embray

  • Milestone changed from sage-8.8 to sage-8.9

Not in Sage 8.8. Let's please to try keep tickets' milestones related to the release in which we actually intend to include them, and in particular the release in which they were actually included, especially when closing tickets.

Note: See TracTickets for help on using tickets.