experimental package for the 3D printing of Cayley graphs.

[kjcollins]

On the website reflections.swarthmore.edu, we have implemented a model builder for Cayley graphs of low-rank reflection groups in a series of SageCells?. We would like to use this code to create an experimental package.

[kjcollins]

Package code currently at this link: ​https://github.com/kjcollins/3d_cayley_graphs. Model class is in cayley_model.sage.

We would like to include this as an addition to sage/plot/plot3d, if it's appropriate.

[kjcollins]

New commits:

​80faffa

uploading package to ticket

[jipilab]

In order for the bot to test your ticket, it is required to have the field "Authors" filled with the full names of the authors

[jipilab]

Hi,

I had a look at the code, here are already some things to have a look at:

• The module should follow the general coding convention of sage, for example the indentation:

EXAMPLES:

Basic plot of a reflection group::     <---- double colon here.
             <------ line break here
    sage: ReflectionGroup(['A',3])                             # optional - gap3
    Irreducible real reflection group of rank 3 and type A3
    sage: w = ReflectionGroup(['A',3])                         # optional - gap3
    sage: ReflectionGroup3d(w)
    Rigid graphical representation of Irreducible real reflection group of rank 3 and type A3
    sage: g = ReflectionGroup3d(w)
    sage: g.plot3d()
    Graphics3d Object

for more details, see here ​https://doc.sagemath.org/html/en/developer/#writing-code-for-sage . Further, the first line of test above is not needed.

• The code should also try to follow pep8 conventions: ​https://www.python.org/dev/peps/pep-0008/

• The examples given in the documentation should work out-of-the-box, that is, running sage -t sage/src/sage/plot/plot3d/reflection_group3d.py should not return failures (currently has 120, for example, you should not forget to import the relevant function like ReflectionGroup in the preamble of the file.)

• The name of the class is perhaps not so well-chosen, perhaps cayley_graph3d? Which leads to a potential follow-up question: could this work eventually for general groups?

• General question: do you really need gap3? You can require gap3, but then should be nice to the user and say it at in the docstring of the module.

• Sage does not usually privilege the usage of user warnings. It should be explained in the docstring of the function what is done, with an example.

• About the Jmol bug, could you give a minimal working instance of the bug?

That's what I can see for now. If you could work on the above list, then I can continue to give feedback once new versions come along!

Best, JP

[tscrim]

Is there something specific about ReflectionGroup that you need to use? Why not WeylGroup or CoxeterGroup (when the Cartan/Coxeter? type is a finite type)?

[kdilks]

New commits:

​b4b9a58	Merge branch 'u/kjcollins/3d_cayley_graphs' of git://trac.sagemath.org/sage into 24280CayGraph
​c62a792	Addressing reviewer changes, one doctest is spitting out magic doctest errors before giving right result.

[kdilks]

The problem is that having

warnings.simplefilter("always")

which is meant to bring up warnings for 3 specific doc-tests is causing a normal doc-test to throw hundreds of warnings. Removing that import just causes those 3 specific doc-tests to return a blank line instead of a warning, so that 3 doc-tests will just be modified/removed.

[git]

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

​798dd74

Pushing changes from my computer since it's not working in the cloud

[jipilab]

Hi,

without the gap3 optional package, I still get 3 doctest failures:

File "reflection_group3d.py", line 63, in sage.plot.plot3d.reflection_group3d
Failed example:
    g_plot = cayley_graph_3d(A1A1, point=(21,11,31))
Expected nothing
Got:
    doctest:warning


File "reflection_group3d.py", line 212, in sage.plot.plot3d.reflection_group3d.cayley_graph_3d._real_dimension
Failed example:
    A = cayley_graph_3d(W)

File "reflection_group3d.py", line 213, in sage.plot.plot3d.reflection_group3d.cayley_graph_3d._real_dimension
Failed example:
    A.real_dimension
Exception raised:

The last two are caused by a missing flag # optional - gap3.

Question: are those warnings _really_ important? This is not typical to have warnings issued to the user like this in Sage.

Instead, perhaps writing a warning block inside of the class to tell the user what the hidden behavior is might be a good idea, as suggested by the Sage Developer's manual:

​https://doc.sagemath.org/html/en/developer/coding_basics.html#documentation-strings

[git]

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

​19dbf74	Merge branch 'u/kdilks/3d_cayley_graphs' of git://trac.sagemath.org/sage into 24280cayley3d
​e1ee29f	All tests are now passing on my cocalc version of sage devel, both with and without the optional gap3. The user warnings have been removed and a warning block about requiring gap3 has been added.