Kappa-Dev/KappaTools

Better documentation for `kappa-library`

jonathan-laurent opened this issue · 1 comments

The documentation for kappa-library is lacking in many places and it would be great if we can launch a collective, incremental effort to improve it. I would be happy to submit PRs myself every time I get explained something or end up figuring it out by myself.

I am not talking about doing anything fancy here but simply making sure that all public-facing API functions are properly explained: what are the arguments (when nonobvious from the type), what is the returned value, what exceptions could be thrown, what are other related functions to be aware of...

For example, I am struggling right now with the absence of documentation for the following function in Edges.mli:

val get_connected_component : int -> t -> int option

Why does this return an option? When is this function expected to return None? Can it also raise exceptions (i.e. if connected components are not supported or the agent ID is invalid)?

Old issue, so not sure if you are still interested in this @jonathan-laurent

There's a lot of missing doc, I'm adding some incrementally on what I find the less clear. Still at the moment, you kinda have to dig in the code to understand how it goes

let get_connected_component ag graph =
  match graph.tables with
  | None -> assert false
  | Some tables ->
    (match tables.connected_component with
    | None ->
      raise
        (ExceptionDefn.Internal_Error
           (Loc.annot_with_dummy
              "get_connected_component while not tracking ccs"))
    | Some ccs -> Mods.DynArray.get ccs ag)

Here, there is indeed both an exception mechanism, an assert and an option (which comes if the agent isn't present in the css DynArray). It might make more sense to unify all these to a exception or an option, but I would need to understand why these choices were made, and how it was done across the surrounding code.

But then, I'm not fond of taking time to document this if that will change in a future revamp/cleanup.

However this poses the question of what is public-facing in the library here. To me it's not clear what can be safely changed, as it might break some code like the one you were writing…

What do you consider public-facing here? All mlis in core? You might have a clearer view than me