Feature edge_info=none #147
Conversation
Updated _generate_dot_graph with edge_info == "none"
Updated _generate_dot_graph with edge_info == "none"
Update parameter description for documentation
Updated parameter description for documentation
Update parameter description for documentation
Update parameter description for documentation
| @@ -190,11 +190,9 @@ def _merge_nodes(self, nodes_to_merge: List[Tuple[str]]): | |||
|
|
|||
| def dot_graph(self, edge_info: str = "probability") -> pdp.Dot: | |||
There was a problem hiding this comment.
| def dot_graph(self, edge_info: str = "probability") -> pdp.Dot: | |
| def dot_graph(self, edge_info: Literal["count", "prior", "posterior", "probability"] | None = "probability") -> pdp.Dot: |
Literal["count", "prior", "posterior", "probability"] -- this says that only "count", "prior", "posterior" or "probability" are acceptable input parameters.
| -- this means "or"
None -- None is its own type in Python, and so we shouldn't be using "none" as a string.
With the above setting, the user will need to explicitly pass None for no information to be displayed.
Alternatively, we can make None the default, so the user needs to explicitly pass one of the acceptable string entries for any information to be displayed.
What do you think?
| for (src, dst, label), attribute in edge_info_dict.items(): | ||
| if edge_info == "count": | ||
| edge_details = str(label) + "\n" + str(attribute) | ||
| elif edge_info == "none": | ||
| edge_details = str(label) | ||
| else: | ||
| edge_details = f"{label}\n{float(attribute):.2f}" |
There was a problem hiding this comment.
| for (src, dst, label), attribute in edge_info_dict.items(): | |
| if edge_info == "count": | |
| edge_details = str(label) + "\n" + str(attribute) | |
| elif edge_info == "none": | |
| edge_details = str(label) | |
| else: | |
| edge_details = f"{label}\n{float(attribute):.2f}" | |
| for (src, dst, label), attribute in edge_info_dict.items(): | |
| if not edge_info: | |
| edge_details = str(label) | |
| elif edge_info == "count": | |
| edge_details = str(label) + "\n" + str(attribute) | |
| else: | |
| edge_details = f"{label}\n{float(attribute):.2f}" |
These are the changes we'd need to make if using None. The truth value of a None type is always False and for a non-None type will be True. So checking it as if not edge_info will only be True when edge_info is None. Let me know if doesn't make sense and I'll give a better example
There was a problem hiding this comment.
Another option is to do this:
edge_details = str(label)
for (src, dst, label), attribute in edge_info_dict.items():
if edge_info == "count":
edge_details = edge_details + "\n" + str(attribute)
else:
edge_details = f"{edge_details}\n{float(attribute):.2f}"
Since edge_details always include str(label), you can add initialise the edge_details variable with the label and then don't need to explicitly check for the None case.
There was a problem hiding this comment.
Same comments as in src/cegpy/graphs/_ceg.py
There was a problem hiding this comment.
Same comments as in src/cegpy/graphs/_ceg.py
|
Good point about not using 'none'. How about changing to 'no info' instead? I think keeping the edge counts/probabilities as default is best - I just wanted the 'no info' option as a niche case where you display a CEG without referencing a specific dataset |
Added the option to include no edge info when drawing et, st or ceg (so that event description is displayed, but edge counts are not). Tested the change locally and all works well.
Also updated the parameter lists to reflect this change to appear in the documentation (and made the edge_info description uniform over different functions).