Opened 3 years ago

Closed 3 years ago

#26658 closed enhancement (fixed)

clean (part 7) - planarity

Reported by: dcoudert Owned by:
Priority: major Milestone: sage-8.5
Component: graph theory Keywords: py3, graph
Cc: tscrim, chapoton Merged in:
Authors: David Coudert Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: b79c678 (Commits, GitHub, GitLab) Commit: b79c6784602c6deb2f3b3b15b7fa07068f963732
Dependencies: Stopgaps:

Status badges


Here we clean methods related to planarity: is_planar, is_circular_planar, layout_planar, is_drawn_free_of_edge_crossings, genus, crossing_number, faces, num_faces, planar_dual.

  • PEP8 cleaning
  • avoid using .edges, or use .edges(sort=False)

Not done:

  • set_planar_positions: it is written inside the method: This method is deprecated since Sage-4.4.1.alpha2. Please use instead .layout`. However, there is no clear deprecation warning. We can either add a proper deprecation warning and remove it in 1 year, or simply remove it as it has been marked as deprecated for so many years... A specific ticket is needed anyway.

Change History (6)

comment:1 Changed 3 years ago by dcoudert

  • Branch set to public/26658_generic_graph_part_7_planarity
  • Cc tscrim chapoton added
  • Commit set to 720f95f974d958bf893be1b14bb47f70ff15d292
  • Status changed from new to needs_review

New commits:

720f95ftrac #26658: clean part 7 - planarity methods

comment:2 Changed 3 years ago by tscrim

  • Reviewers set to Travis Scrimshaw

Another nitpick and while-we-are-at-it: we should not use contractions in error messages (can't) to be more formal. However, if you do not want to change that, you can set a positive review.

comment:3 Changed 3 years ago by git

  • Commit changed from 720f95f974d958bf893be1b14bb47f70ff15d292 to b79c6784602c6deb2f3b3b15b7fa07068f963732

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

b79c678trac #26658: can't -> cannot

comment:4 Changed 3 years ago by dcoudert

  • Summary changed from clean connectivity.pyx (part 7) - planarity to clean (part 7) - planarity

I was not sure if can't was ok or not. Now I know and so I followed your recommendation.

comment:5 Changed 3 years ago by tscrim

  • Status changed from needs_review to positive_review

I don't think there is an official policy, but it just makes sense in terms of formality (e.g., you would not write it in a paper). Thank you for also fixing it.

comment:6 Changed 3 years ago by vbraun

  • Branch changed from public/26658_generic_graph_part_7_planarity to b79c6784602c6deb2f3b3b15b7fa07068f963732
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.