Fix Generalized Petersen graph name and update others to f-string syntax #40036
Conversation
|
Documentation preview for this PR (built with commit a226b50; changes) is ready! 🎉 |
|
Please add a doctest showing what has been fixed. |
| @@ -1711,6 +1711,13 @@ def GeneralizedPetersenGraph(n, k): | |||
| sage: g.is_bipartite() | |||
| True | |||
|
|
|||
| TESTS:: | |||
There was a problem hiding this comment.
only one colon in this line
There was a problem hiding this comment.
That's confusing, because I copied the syntax from elsewhere in the file. E.g. in the definition of MathonPseudocyclicMergingGraph (line 3829) SEEALSO and TESTS both have double-colons. There are ~20 other places with TESTS::. Are these also docstring bugs that I should fix while I'm working on this file?
There was a problem hiding this comment.
double colon means that it is followed by an indented block. Here it is not because you have another line with the same indent, itself ending with a double colon and followed by its indented block. So : just put one single colon at the end of TESTS on the one line that I designated
|
Thank you for the quick review. I have to say that since I got my first small PR into sage in 2019 the developer experience has improved immensely. Github is easier for me than trac ever was, and the new conda install methods for both using sage and developing sage are awesome compared to compiling from source. Such amazing work by the community over the years. |
sagemathgh-40036: Fix Generalized Petersen graph name and update others to f-string syntax There is a bug in the naming of the generalized Petersen graphs: the parameters aren't interpolated into the name string properly. This PR fixes that bug and updates all names in `sage.graphs.generators.families` to use f-strings. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies URL: sagemath#40036 Reported by: Ewan Davies Reviewer(s): Ewan Davies, Frédéric Chapoton
sagemathgh-40036: Fix Generalized Petersen graph name and update others to f-string syntax There is a bug in the naming of the generalized Petersen graphs: the parameters aren't interpolated into the name string properly. This PR fixes that bug and updates all names in `sage.graphs.generators.families` to use f-strings. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies URL: sagemath#40036 Reported by: Ewan Davies Reviewer(s): Ewan Davies, Frédéric Chapoton
sagemathgh-40036: Fix Generalized Petersen graph name and update others to f-string syntax There is a bug in the naming of the generalized Petersen graphs: the parameters aren't interpolated into the name string properly. This PR fixes that bug and updates all names in `sage.graphs.generators.families` to use f-strings. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies URL: sagemath#40036 Reported by: Ewan Davies Reviewer(s): Ewan Davies, Frédéric Chapoton
There is a bug in the naming of the generalized Petersen graphs: the parameters aren't interpolated into the name string properly. This PR fixes that bug and updates all names in
sage.graphs.generators.familiesto use f-strings.📝 Checklist
⌛ Dependencies