Opened 6 years ago

Closed 6 years ago

#18045 closed defect (fixed)

Wrong result returned by is_planar on a given embedding

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.6
Component: graph theory Keywords:
Cc: Merged in:
Authors: Nathann Cohen Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: 110c8fa (Commits, GitHub, GitLab) Commit: 110c8fa7f74010a01187f1413e2e693868cf1a66
Dependencies: Stopgaps:

Status badges

Description (last modified by ncohen)

As reported on AskSage [1], the function .is_planar can return wrong results when given a specific embedding to run its computations on.

This actually comes from a mistake in a if/else and variable types, as bool(a_dictionary) returns True when the dictionary is nonempty. The original authors did not seem to be aware of that, and once fixed the code does not make the mistake again.

The problem was that the computations were run on the cached embedding (i.e _embedding) instead of the one provided by the user.

Nathann

[1] ​http://ask.sagemath.org/question/26301/testing-planarity-on-embedding-gives-wrong-result/

Change History (11)

comment:1 Changed 6 years ago by ncohen

  • Branch set to public/18045
  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 6 years ago by ncohen

  • Description modified (diff)

comment:3 Changed 6 years ago by git

  • Commit set to 38c4e50b38574c6b4be2bb9dbf3cd98b1d4f6eaf

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

​38c4e50trac #18045: Wrong result returned by is_planar on a given embedding

comment:4 Changed 6 years ago by git

  • Commit changed from 38c4e50b38574c6b4be2bb9dbf3cd98b1d4f6eaf to 9ee0d6e19ed03501a03333dfa4815f947735c0bc

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

​9ee0d6etrac #18045: Wrong result returned by is_planar on a given embedding

comment:5 Changed 6 years ago by vdelecroix

  • Reviewers set to Vincent Delecroix

One bug less in Sage! Cool!

Could you replace (see eg's) by (see examples). It is very wrong as "e.g." is a latin abbreviation for "for example".

Vincent

comment:6 Changed 6 years ago by vdelecroix

  • Status changed from needs_review to needs_work

comment:7 Changed 6 years ago by git

  • Commit changed from 9ee0d6e19ed03501a03333dfa4815f947735c0bc to 110c8fa7f74010a01187f1413e2e693868cf1a66

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

​69640d6trac #18045: Merged with 6.7.beta2
​110c8fatrac #18045: Review

comment:8 Changed 6 years ago by ncohen

  • Status changed from needs_work to needs_review

comment:9 Changed 6 years ago by vdelecroix

  • Status changed from needs_review to positive_review

comment:10 Changed 6 years ago by ncohen

Thanks !

comment:11 Changed 6 years ago by vbraun

  • Branch changed from public/18045 to 110c8fa7f74010a01187f1413e2e693868cf1a66
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.