Opened 4 years ago

Closed 4 years ago

#18775 closed enhancement (fixed)

polytopes.icosidodecahedron and graphs.TruncatedIcosidodecahedralGraph

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.8
Component: geometry Keywords: graph, polytope
Cc: vdelecroix, dimpase Merged in:
Authors: Nathann Cohen Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: a4b9287 (Commits) Commit: a4b9287f3e66be106825f090c55ffec7449532ef
Dependencies: Stopgaps:

Change History (14)

comment:1 Changed 4 years ago by ncohen

  • Branch set to public/18775
  • Commit set to 64b689bcb3fe9370bfebd23edefb9148401c3cb2
  • Status changed from new to needs_review

New commits:

64b689btrac #18775: polytopes.icosidodecahedron and graphs.TruncatedIcosidodecahedralGraph

comment:2 Changed 4 years ago by git

  • Commit changed from 64b689bcb3fe9370bfebd23edefb9148401c3cb2 to 9fa4890751b70c1fa2d45573f6e74a770593dd03

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

9fa4890trac #18775: now with TruncatedTetrahedralGraph

comment:3 Changed 4 years ago by ncohen

  • Cc dimpase added

For some reason the wikipedia page about the truncated tetrahedron reports 120 automorphisms, and Sage computes 24. I will try to figure this out.

Nathann

comment:4 follow-up: Changed 4 years ago by chapoton

I have done the Icosidodecahedron in #18225...

comment:5 in reply to: ↑ 4 Changed 4 years ago by ncohen

I have done the Icosidodecahedron in #18225

Oh. Well, then we will update one of the two tickets once the other is merged. I don't care where it comes from, I just want to have it in Sage.

Nathann

comment:6 Changed 4 years ago by ncohen

So what do we do?

We can: 1) Review+merge this one and adapt yours 2) Review+merge yours (much longer, as you have more code) and adapt mine

Nathann

comment:7 follow-up: Changed 4 years ago by chapoton

I have found a typo: in the line

+    The truncated icosidodecahedron is an Archimedean solid with 12 vertices and

it should be "truncated tetrahedon".

And may I request a little pep8 effort ?

comment:8 Changed 4 years ago by git

  • Commit changed from 9fa4890751b70c1fa2d45573f6e74a770593dd03 to a4b9287f3e66be106825f090c55ffec7449532ef

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

54f4ff5trac #18775: Merged with 6.9.beta0
a4b9287trac #18775: typo

comment:9 in reply to: ↑ 7 Changed 4 years ago by ncohen

it should be "truncated tetrahedon".

Thanks, fixed.

And may I request a little pep8 effort ?

I have no idea what pep8 is about, and I do not intend to read (nor to follow) a document that tells me where I should put spaces before/after the '=' signs. Call me careless, but I find tickets #18904 way more important/urgent. Why people care more about spaces than things like that is beyond me.

Nathann

comment:10 follow-up: Changed 4 years ago by chapoton

  • Status changed from needs_review to positive_review

ok, I understand.

Positive review.

I you have some time for a quick review, there is #10922 which is just about doc formatting.

comment:11 Changed 4 years ago by chapoton

by the way, what do you think of the new patchbot icons ? click on top right round icon to see some of them..

comment:12 Changed 4 years ago by chapoton

  • Keywords graph polytope added
  • Reviewers set to Frédéric Chapoton

comment:13 in reply to: ↑ 10 Changed 4 years ago by ncohen

Helloooo,

Positive review.

Thanks!

I you have some time for a quick review, there is #10922 which is just about doc formatting.

In a second.

by the way, what do you think of the new patchbot icons ? click on top right round icon to see some of them..

Oh. I had not noticed. Well, pretty and more informative :-)

Nathann

comment:14 Changed 4 years ago by vbraun

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