Opened 6 years ago

Closed 6 years ago

#20816 closed enhancement (fixed)

pep8 in magma interface

Reported by: Frédéric Chapoton Owned by:
Priority: major Milestone: sage-7.3
Component: interfaces: optional Keywords: magma
Cc: Travis Scrimshaw, Jeroen Demeyer Merged in:
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: cd6daf2 (Commits, GitHub, GitLab) Commit: cd6daf2a45bcf981f04d7c95ede61ee30c27b116
Dependencies: Stopgaps:

Status badges

Description

A little cleanup of interfaces/magma

and one change to a more stable doctest

(so that all tests pass)

Change History (7)

comment:1 Changed 6 years ago by Frédéric Chapoton

Branch: u/chapoton/20816
Commit: cd6daf2a45bcf981f04d7c95ede61ee30c27b116
Status: newneeds_review

New commits:

cd6daf2full pep8 in magma interface, and one more stable doctest

comment:2 Changed 6 years ago by Frédéric Chapoton

Cc: Travis Scrimshaw Jori Mäntysalo Jeroen Demeyer added

comment:3 Changed 6 years ago by Jori Mäntysalo

Cc: Jori Mäntysalo removed

I got no magma, so I'll un-cc me.

comment:4 Changed 6 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw

LGTM modulo this change:

-            sage: v = V.list_attributes(); v.sort(); v               # optional - magma
-            ['Coroots', 'Involution', 'M', 'RootDatum', 'Roots', 'StrLocalData', 'T', 'decomp', 'eisen', 'exthom', 'hSplit', 'ip_form', 'p', 'ssbasis', 'weights']
+            sage: v = V.list_attributes(); v.sort()               # optional - magma
+            sage: print(v)     # optional - magma
+            ['Coroots', 'Involution', ..., 'p', 'ssbasis', 'weights']

Is there a reason why we don't keep all of them (I don't have magma so I can't check)? Also, the print is vacuous to me.

comment:5 in reply to:  4 Changed 6 years ago by Frédéric Chapoton

Replying to tscrim:

LGTM modulo this change:

-            sage: v = V.list_attributes(); v.sort(); v               # optional - magma
-            ['Coroots', 'Involution', 'M', 'RootDatum', 'Roots', 'StrLocalData', 'T', 'decomp', 'eisen', 'exthom', 'hSplit', 'ip_form', 'p', 'ssbasis', 'weights']
+            sage: v = V.list_attributes(); v.sort()               # optional - magma
+            sage: print(v)     # optional - magma
+            ['Coroots', 'Involution', ..., 'p', 'ssbasis', 'weights']

Is there a reason why we don't keep all of them (I don't have magma so I can't check)? Also, the print is vacuous to me.

Yes, this is the core of this ticket, making this precise doctest more robust by allowing some changes in the list of attributes. I ran the doctest on a machine with the latest magma and it was failing because of the presence of two more attributes. The aim of the test is not to check that we are up-to-date with a precise version of magma, but that we can indeed manage to get the attributes from magma itself.

comment:6 Changed 6 years ago by Travis Scrimshaw

Status: needs_reviewpositive_review

Okay, thanks. Then in it goes.

comment:7 Changed 6 years ago by Volker Braun

Branch: u/chapoton/20816cd6daf2a45bcf981f04d7c95ede61ee30c27b116
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.