Opened 4 years ago

Closed 4 years ago

#26658 closed enhancement (fixed)

clean generic_graph.py (part 7) - planarity

Reported by: David Coudert Owned by:
Priority: major Milestone: sage-8.5
Component: graph theory Keywords: py3, graph
Cc: Travis Scrimshaw, Frédéric 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

Description

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 4 years ago by David Coudert

Branch: public/26658_generic_graph_part_7_planarity
Cc: Travis Scrimshaw Frédéric Chapoton added
Commit: 720f95f974d958bf893be1b14bb47f70ff15d292
Status: newneeds_review

New commits:

720f95ftrac #26658: clean generic_graph.py part 7 - planarity methods

comment:2 Changed 4 years ago by Travis Scrimshaw

Reviewers: 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 4 years ago by git

Commit: 720f95f974d958bf893be1b14bb47f70ff15d292b79c6784602c6deb2f3b3b15b7fa07068f963732

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

b79c678trac #26658: can't -> cannot

comment:4 Changed 4 years ago by David Coudert

Summary: clean connectivity.pyx (part 7) - planarityclean generic_graph.py (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 4 years ago by Travis Scrimshaw

Status: needs_reviewpositive_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 4 years ago by Volker Braun

Branch: public/26658_generic_graph_part_7_planarityb79c6784602c6deb2f3b3b15b7fa07068f963732
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.