Skip to content

clean generic_graph.py (part 7) - planarity #26658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dcoudert opened this issue Nov 7, 2018 · 10 comments
Closed

clean generic_graph.py (part 7) - planarity #26658

dcoudert opened this issue Nov 7, 2018 · 10 comments

Comments

@dcoudert
Copy link
Contributor

dcoudert commented Nov 7, 2018

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.

CC: @tscrim @fchapoton

Component: graph theory

Author: David Coudert

Branch/Commit: b79c678

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/26658

@dcoudert dcoudert added this to the sage-8.5 milestone Nov 7, 2018
@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 7, 2018

New commits:

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

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 7, 2018

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 7, 2018

Commit: 720f95f

@tscrim
Copy link
Collaborator

tscrim commented Nov 8, 2018

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Nov 8, 2018

comment:2

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 8, 2018

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

b79c678trac #26658: can't -> cannot

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 8, 2018

Changed commit from 720f95f to b79c678

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 8, 2018

comment:4

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

@dcoudert dcoudert changed the title clean connectivity.pyx (part 7) - planarity clean generic_graph.py (part 7) - planarity Nov 8, 2018
@tscrim
Copy link
Collaborator

tscrim commented Nov 8, 2018

comment:5

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.

@vbraun
Copy link
Member

vbraun commented Nov 11, 2018

Changed branch from public/26658_generic_graph_part_7_planarity to b79c678

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants