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

Simplify SuccessionDiagram API #93

Open
jcrozum opened this issue Jan 25, 2024 · 2 comments
Open

Simplify SuccessionDiagram API #93

jcrozum opened this issue Jan 25, 2024 · 2 comments

Comments

@jcrozum
Copy link
Owner

jcrozum commented Jan 25, 2024

While building the docs, I noticed that the SuccessionDiagram API could be simplified in a few ways:

  1. We have various node attributes that could be grouped into a separate object, e.g., sd.node_is_expanded should be sd.node.is_expanded.
  2. Several of the from_xxx(s) methods could be grouped into one function, e.g., from_string(s, format=xxx).
  3. The various expansion methods could be aggregated into one with an method argument.
@jcrozum
Copy link
Owner Author

jcrozum commented Jan 25, 2024

One more thing to note about this class is that there is a circular import issue, and there are some troubles importing the class from the top level. We should rename SuccessionDiagram.py to succession_diagram.py and move the SuccessionDiagram import in the SCC expansion to within the function call (or even better, come up with a good way to avoid needing the import at all).

@jcrozum
Copy link
Owner Author

jcrozum commented Mar 1, 2024

Note #106 addresses points 1 & 2 above, but not 3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant