Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#23300 closed defect (fixed)

non-absolute import in BasisExchangeMatroid

Reported by: Stefan Owned by:
Priority: major Milestone: sage-8.0
Component: matroid theory Keywords: import, isomorphism, matroids
Cc: Rudi, yomcat, zgershkoff Merged in:
Authors: Stefan van Zwam Reviewers: Zach Gershkoff, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: b443da7 (Commits) Commit:
Dependencies: Stopgaps:

Description

The following should not give an error:

sage: N = Matroid(field=GF(3), reduced_matrix=[[1,1],[1,-1]])
sage: M = matroids.Uniform(2,4)
sage: N._isomorphism(M)

The problem is a non-absolute import statement.

Change History (9)

comment:1 Changed 2 years ago by Stefan

  • Branch set to u/Stefan/non_absolute_import_in_basisexchangematroid

comment:2 Changed 2 years ago by Stefan

  • Authors set to Stefan van Zwam
  • Commit set to 221bf3025abab9508edaf3afebfd211ab814ee5f
  • Status changed from new to needs_review

Fixed this. Turns out another instance occurred in the _is_isomorphism method in the same file, which I also fixed.


New commits:

221bf30fixed import statements and added tests

comment:3 Changed 2 years ago by zgershkoff

  • Status changed from needs_review to positive_review

The methods work without errors when I compare basis exchange matroids to rank matroids, circuit closure matroids, and prototype graphic matroids.

comment:4 Changed 2 years ago by tscrim

  • Status changed from positive_review to needs_work

Reviewer name.

However, I would say a better (well, at least cleaner IMO) idiom is to just import what you need

-            from . import basis_matroid
-            ot = basis_matroid.BasisMatroid(other)
+            from .basis_matroid import BasisMatroid
+            ot = BasisMatroid(other)

comment:5 Changed 2 years ago by git

  • Commit changed from 221bf3025abab9508edaf3afebfd211ab814ee5f to b443da71a73159fa2cf364f2f0da54b4000ccb6d

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

b443da7modified imports according to trac comment

comment:6 Changed 2 years ago by Stefan

  • Status changed from needs_work to needs_review

Followed Travis' suggestion

comment:7 Changed 2 years ago by zgershkoff

  • Reviewers set to Zachary Gershkoff, Travis Scrimshaw
  • Status changed from needs_review to positive_review

Still works.

comment:8 Changed 2 years ago by vbraun

  • Branch changed from u/Stefan/non_absolute_import_in_basisexchangematroid to b443da71a73159fa2cf364f2f0da54b4000ccb6d
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:9 Changed 2 years ago by jdemeyer

  • Commit b443da71a73159fa2cf364f2f0da54b4000ccb6d deleted
  • Reviewers changed from Zachary Gershkoff, Travis Scrimshaw to Zach Gershkoff, Travis Scrimshaw
Note: See TracTickets for help on using tickets.