Open daemontus opened 3 years ago
This is a much bigger issue than it may initially seem. The problem is largely a bad design in the original API, where a lot of operations that can fail have no way to return an error. So the only option at the moment is to panic. Also, a lot of checks which are performed by the parser should in fact be performed by BooleanNetwork
and RegulatoryGraph
instead, otherwise one can easily create a malformed network using our current API.
An (incomplete) list of problems that need to be addressed:
RegulatoryGraph::new
should check if variables are unique and that their names are valid.RegulatoryGraph::remove_regulation
should be available. Maybe a "force" variant can be also added, which removes the variable from other update functions if used.RegulatoryGraph::add_variable
and RegulatoryGraph::remove_variable
should be also available, again with all the necessary safety checks. Variable ID can be reused once deleted.BooleanNetwork
should be also able to remove and add parameters, and update their arity.BooleanNetwork::set_update_function
must verify that the update function is valid within the network at the time of insertion. String
as error type to some custom enum with a sensible to_string()
implementation. Other tools may want to display our errors differently.
Public methods for manipulation of Boolean networks were originally designed just for the
.aeon
format parser. As such, they are missing some safety checks (like verifying that an update function actually uses parameters which are present in the network).At least, we should fix those safety issues (every change to the network should probably trigger some kind of full consistency check). Additionally, it would be good to design the API so that a BN can be modified more easily. For example, getting a mutable reference of the inner regulatory graph would help a lot (since then we can add regulations even after a BN is constructed from the graph).