Opened 5 years ago

Closed 5 years ago

#17656 closed defect (fixed)

More meaningful error messages from _check_embedding_validity

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.5
Component: graph theory Keywords:
Cc: Merged in:
Authors: Nathann Cohen Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: a0f21af (Commits) Commit: a0f21afee1708c431d26894ee0531f74b765951b
Dependencies: Stopgaps:

Description

It has been reported [1] on sage-devel by Nikos Apostolakis that _check_embedding_validity was rather lacking in its checks: it did not notice that some vertex appeared twice in a circular ordering.

With this branch, more meaningful exceptions are raised when something goes wrong, and in particular his example of code now stops at this moment: 

sage: k12.set_embedding(imb)
...
ValueError: The list associated with vertex A contains >1 occurrences of: [C]

Nathann

[1] ​https://groups.google.com/d/topic/sage-support/TsXzNtwuPvo/discussion

Change History (7)

comment:1 Changed 5 years ago by ncohen

  • Branch set to u/ncohen/17656
  • Status changed from new to needs_review

comment:2 Changed 5 years ago by git

  • Commit set to 390594da5b9652ceac9019b4d6c9c8a0bbdb7cf8

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

​390594dtrac #17656: More meaningful error messages from _check_embedding_validity

comment:3 Changed 5 years ago by git

  • Commit changed from 390594da5b9652ceac9019b4d6c9c8a0bbdb7cf8 to 44b5b5d9f961429cfcfebc396a210eabfc347b27

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

​44b5b5dtrac #17656: Merged with 6.7.beta2

comment:4 Changed 5 years ago by git

  • Commit changed from 44b5b5d9f961429cfcfebc396a210eabfc347b27 to a0f21afee1708c431d26894ee0531f74b765951b

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

​a0f21aftrac #17656: Merged with 6.7.beta4

comment:5 Changed 5 years ago by chapoton

  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_review to positive_review

ok, looks good (even if not at the pep8 standard)

comment:6 Changed 5 years ago by ncohen

Thaaaaaaaaaaaaaaanks ! ;-)

Nathann

comment:7 Changed 5 years ago by vbraun

  • Branch changed from u/ncohen/17656 to a0f21afee1708c431d26894ee0531f74b765951b
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.