Transition from Requires.jl to v1.9-style weak dependencies

[ffevotte]

Now that 1.9 allows for "weak dependencies", I wanted to test whether we could use these to handle optional dependencies to GraphViz & Makie for visualization.

This is (the beginning of) an attempt to do this. It's not finished (yet), but I wanted to discuss this as early as possible in order to make an informed decision about where we go with this.

One first thing to note is that it is nice to have these TTFX-heavy workloads inside precompileable modules. The drawback is that these modules are "insulated" from DataFlowTasks: the visualization code used to be able to use anything from DataFlowTasks, and also used to be able to define new functions inside the DataFlowTasks namespace. This is not the case anymore; the good practice seems to use extension code to add methods to existing functions, which requires our old Requires-based code to be updated in one of two ways (there might also be more ways to do things that I haven't thought about):

1. extend functions from the public API of Makie/GraphViz. For example, I replaced our custom plot_traces and plot_dag with new methods for Makie.plot and GraphViz.Graph. Below is an excerpt from the updated README (other examples are not up-to-date):

   log_info = DataFlowTasks.@log cholesky_dft!(A ,ts);
   dag = GraphViz.Graph(log_info)                               # formerly plot_dag(log_info)
   trace = plot(log_info; categories=["chol", "ldiv", "schur"]) # formerly plot_traces(log_info; categories)

2. extend functions from DataFlowTasks itself. This means that DataFlowTasks might define functions which are available but unusable at first because they have no method until an extension is loaded. I did this for example with the utility function savedag, which unfortunately does not have an equivalent in GraphViz's public API.

Another thing to take care of is to make sure that unit tests can use functions internally defined in the extensions. This adds a few lines at the top of loginfo_test.jl, but it also provides a clean way to check whether extensions are actually loaded, so all in all I'm rather happy with this.

So I guess the next questions are:

1. do we want to go along this path and declare GraphViz and Makie as weak dependencies? The associated TODO list would IMO mostly involve adapting the documentation and examples to the new API (with a bit of a question mark related to how Documenter handles extensions)
2. do we want to keep support for Requires-based optional dependencies for users of julia v1.8 and below? It looks like this is feasible, but it would involve a few extra steps.
3. Are we happy with the accompanying API changes? It would be relatively easy to revert them and keep the original API, defining empty functions in DataFlowTasks that would not be usable until extensions are loaded.

[maltezfaria]

I like the move towards WeakDeps, and the precompilation benefits that come with it!

I think the "drawbacks", if you can call it that, of having to either extend methods of the loaded extension or of the main package, are nothing really major. I don't see any pain points there.

As for providing backward compatibility with pre 1.9 Julia versions, I don't think we should care too much for now. We can always fix this later if needed.

[codecov]

Codecov Report

Merging #42 (e7d5561) into main (eecc3db) will increase coverage by 0.17%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
+ Coverage   72.42%   72.59%   +0.17%     
==========================================
  Files          10       10              
  Lines         747      748       +1     
==========================================
+ Hits          541      543       +2     
+ Misses        206      205       -1     

Impacted Files	Coverage Δ
src/logger.jl	78.57% <ø> (ø)
ext/DataFlowTasks_GraphViz_Ext.jl	84.00% <75.00%> (ø)
ext/DataFlowTasks_Makie_Ext.jl	90.37% <100.00%> (ø)
src/DataFlowTasks.jl	93.33% <100.00%> (-0.79%)	⬇️

... and 1 file with indirect coverage changes

[ffevotte]

This should be more or less complete now. Since extension modules are not directly available, documenting the interface they introduce proved to be a bit more involved than expected, but all examples and docs should now be up-to-date with the new way of doing things.

One thing we'll have to do at some point before the next release: update Project.toml to

1. specify compat bounds for weak dependencies (I think this is mandatory), and
2. bump the version number (I guess to 0.2.0 to signal a breaking release, even though the breaking parts are in extensions).

[maltezfaria]

LGTM (there is one lingering reference to Requires in the README.jl)

Edit: I went ahead and fixed it, as well as a few typos.

[maltezfaria]

One thing we'll have to do at some point before the next release: update Project.toml to

1. specify compat bounds for weak dependencies (I think this is mandatory), and
2. bump the version number (I guess to 0.2.0 to signal a breaking release, even though the breaking parts are in extensions).

Should we discuss some other (minor) improvements for 0.2? I mentioned some pain points in #40, and would be happy to put in the time to implement a few new features.

[maltezfaria]

I never add the project itself as a dependency to docs, as I was under the impression this was not necessary (Documenter looks into its parent folder for a project right?).

[ffevotte]

I don't think Documenter looks into its parent folder to find the documented project, so at the time when the documentation is built, the parent package has to be present in the documentation environment in some way. However, one of the steps in the GitHub CI action takes care of Pkg.deving the parent project in the documentation subproject. So both ways work: if the committed docs/Project.toml does not mention DataFlowTasks, things will get set up correctly just ahead of time.

I've recently seen packages where even the docs/Manifest.toml was committed so as to make sure that the parent project is deved from the parent directory. Graphs.jl is an example of this.

Personally, I'd be in favor of keeping DataFlowTasks as a documentation dependency in the committed docs/Project.toml, because this simplifies the process to locally build the documentation. But I'm open to compromise 😄

[ffevotte]

Should we discuss some other (minor) improvements for 0.2? I mentioned some pain points in #40, and would be happy to put in the time to implement a few new features.

I've opened #43 to list the ideas that came to my mind when re-opening this project after a few months.

[maltezfaria]

Can we go ahead and merge this?

[ffevotte]

Yes, we can merge this.

The only unresolved issue is related to the docs dependencies, so nothing specific to the weakdeps transitions and we can decide later. I'm going to mention this in #43 so that we don't forget to decide about this.