Skip to content
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

Fixing failing silently #28

Merged
merged 7 commits into from
Jun 19, 2024
Merged

Conversation

mpan322
Copy link
Contributor

@mpan322 mpan322 commented Jun 5, 2024

For Issue: #20

Possible Failure Cases:

  • Add node
    • if node exists with the same name it should fail.
    • if the node's name is not a valid node identifier it should fail immediately X (at the moment there is only very basic checking done when stringifying) . I am working on code to check based on the graphviz specification.
  • Add edge
    • if either of the nodes do not exist it automatically creates the node. I think this is fine as it is much more convienient.
    • it should also allow for node failure cases above based on their names
  • Remove node
    • if the node does not exist it should fail
  • Remove edge
    • if no such edge exists it should fail
  • Add subgraph / adding a context
    • if there is already a subgraph / context with the same name within the same parent it should fail
      • QUESTION: At the moment this check is only done within the parent i.e. if g has subgraphs a and b it is valid for both a and b to have children labelled c.
      • I am partial to removing this as it will likely make the subgraph tree searching operations difficult to use properly or not make sense in their current version. Let me know if you think there is a good reason to keep this behaviour.
  • Add attribute
    • if no known attribute exists there is a warning (discussed this previously)
  • Remove attribute
    • removing an attribute which is not set causes in error
  • Set attribute (which is already set)
    • overwrites but does not change warn or error (I find this behaviour fine - let me know if you disagree)
    • CHANGE: made it so adding an attribute to a graph now follows the above behaviour (before it was not removed)

Commits

  • Added errors and updated tests for the following cases: - removing edges between nodes which do not exist. - removing attributes which have not been set.
  • 1. Added quotations arround node names in errors. 2. When removing edges between nodes in a graph if no edges exist and error is outputted.
  • - Added function to check if a label is valid. - Reduced some code duplication in other spots TODO: add label validity checking to all functions where labels may be added.
  • added planning
  • added attribute removal when setting an existing attribute on graphs, added more tests and went through operations which could fail and reasonably should error

mpan322 and others added 7 commits May 24, 2024 14:43
 - removing edges between nodes which do not exist.
 - removing attributes which have not been set.
2. When removing edges between nodes in a graph if no edges exist and
   error is outputted.
- Reduced some code duplication in other spots
TODO: add label validity checking to all functions
where labels may be added.
… added more tests and went through operations which could fail and reasonably should error
@james-d-mitchell james-d-mitchell merged commit d908f51 into digraphs:main Jun 19, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants