Skip to content

Conversation

james-d-mitchell
Copy link
Member

This is a reworking of #639, which goes a bit further than #639 in removing the old stuff for displays. This PR doesn't introduce any breaking changes except that we now require the graphviz package. So this PR shouldn't be merged until there is a release of `graphviz'.

@james-d-mitchell
Copy link
Member Author

james-d-mitchell commented May 7, 2024

TODO:

  • doc
  • tests for code coverage

@james-d-mitchell james-d-mitchell added major A label for PRs or issues that are major in some sense. refactor Label for PRs or issues related to refactoring code labels May 8, 2024
@james-d-mitchell james-d-mitchell force-pushed the graphviz branch 10 times, most recently from b35efd9 to d4b6e10 Compare May 10, 2024 12:35
@james-d-mitchell james-d-mitchell force-pushed the graphviz branch 5 times, most recently from 19fe156 to 5416f88 Compare May 15, 2024 12:09
@james-d-mitchell
Copy link
Member Author

The CI will likely fail here until:

digraphs/graphviz#21

is merged.

@james-d-mitchell james-d-mitchell added the gap-days-brussels-2025 Label for things we might work on in Brussels label Apr 7, 2025
@james-d-mitchell
Copy link
Member Author

This might be a bit much for this week, but maybe not.

@james-d-mitchell james-d-mitchell removed the gap-days-brussels-2025 Label for things we might work on in Brussels label Apr 9, 2025
Copy link
Collaborator

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really cool!

@@ -0,0 +1,27 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is autogenerated and is listed in .gitignore and should not be included in the repo.

<#GAPDoc Label="GraphvizVertexLabelledDigraph">
<ManSection>
<Attr Name="GraphvizVertexLabelledDigraph" Arg="D"/>
<Attr Name="GraphvizVertexLabelledGraph" Arg="D"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet, I was wishing we had such a function already yesterday.

<Description>
These functions produce &graphviz; objects representing the digraph
<A>D</A>, where the vertices in the list <A>verts</A>, and edges
between them, are drawn with color <A>color1</A> and all other vertices
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the text of the Digraphs manual, we sometimes use the US spelling color and sometimes we use the British spelling colour. I suggest we standardise at some point.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file can be removed, presumably it's just a vestige?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file can be removed, presumably it's just a vestige?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this file part of this PR? Should it be a separate one?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff looks nice so far, although it's obviously got a few TODOs etc, but this should eventually be its own PR too.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
major A label for PRs or issues that are major in some sense. refactor Label for PRs or issues related to refactoring code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants