Subobject image/preimage, completion of a partial subobject

[kris-brown]

These functionalities for subobjects are code I find myself rewriting repeatedly, so they may be of use to others too (if they're not already in Catlab).

Because I often want to 'apply' a homomorphism to some concrete data, this becomes a map of subobjects from the domain to the codomain. We likewise can use preimages to get maps in the other direction (I'd like a better syntax for inverses, but for now it's a named function hom_inv).

Also, I often want, e.g., the wiring diagram induced by wires 3, 4, and 7 - so induce_subobject will do the iterative search so that the subobject doesn't have 0's everywhere.

[github-actions]

Review Checklist

Does this PR follow the development guidelines? Following is a partial checklist:

Tests

• New features and bug fixes have unit tests
• New modules have tests that are ultimately called by the test runner (test/runtests.jl)
• Existing tests have not been deleted
• Code coverage >= 90% or reduction justified in PR

Documentation

• All exported functions, types, and constants have docstrings, written in complete sentences
• Citations are given for any constructions, algorithms, or code drawn from external sources

Other

• Style guidelines are followed, including indent width 2
• Changes breaking backwards compatibility have been approved

[bosonbaas]

This code looks great, just have a few comments and a question on the docstrings. Looking forward to inducing subobjects!

[bosonbaas]

Looks like the test suite is failing on the new tests (though you may already be aware of that).

[bosonbaas]

This is repeatedly failing on generating the code for one of the docstrings. I'm wondering if one of the added docstrings is unintentionally being interpreted as code for documenter to execute? (error here https://github.com/AlgebraicJulia/Catlab.jl/runs/5409417886?check_suite_focus=true#step:6:97).
@kris-brown have you been able to build the documentation locally for this PR?

[kris-brown]

Hm this error is weird because it seems to be in a completely unrelated file.

For generating docs locally: running docs/make.jl in the environment of that folder, I cannot build docs, even with the master branch of Catlab due to this error:

┌ Error: error when executing notebook based on input file: `~/code/Catlab.jl/docs/literate/graphics/composejl_wiring_diagrams.jl`
└ @ Literate ~/.julia/packages/Literate/kB2Gm/src/Literate.jl:609
ERROR: LoadError: LoadError: MethodError: no method matching SCS.Optimizer(; verbose=false)

Note that error is different from the one github is reporting. For the local error, I think the problem occurs when solve_isotonic calls solve! with the SCS.Optimizer type passed in as an argument. Maybe we need tighter version bounds in the docs/Project.toml?

My environment:

[134e5e36] Catlab v0.13.7
  [5ae59095] Colors v0.12.8
  [a81c6b42] Compose v0.9.3
  [f65535da] Convex v0.15.0
  [864edb3b] DataStructures v0.18.11
  [e30172f5] Documenter v0.25.5
  [682c06a0] JSON v0.21.3
  [b964fa9f] LaTeXStrings v1.3.0
  [98b081ad] Literate v2.8.0
  [08abe8d2] PrettyTables v1.3.1
  [c946c3f1] SCS v1.1.0
  [1e332c56] TikzCDs v0.2.1
  [37f6aa50] TikzPictures v3.4.2

[epatters]

This should be fixed in #607. Try rebasing off master.

[bosonbaas]

So, I think that it's not the actual content of the comments, but instead the fact that there are docstrings on your (f::ACSetTransformation) functions. When I comment out just these docstrings, it compiles fine, but whatever they are, if they're included it breaks

[bosonbaas]

This looks like a similar issue, though it's not the exact same situation (though the same error)
JuliaDocs/Documenter.jl#1192

[bosonbaas]

Sorry it's taken me so long to get back to this! If you remove the return types, the docs will build even with the docstrings

(f::ACSetTransformation)(X::SubACSet)::SubACSet

to

(f::ACSetTransformation)(X::SubACSet)

Not sure if that causes any performance or enforcement issues on Julia's side, but it'll let you write docstrings for these functions. So it currently is just a choice between docstrings and return types from what I can see. Any thoughts on this tradeoff @epatters @kris-brown ?

[epatters]

I'm pretty sure return type annotations just add a runtime type assert for the return value, so removing them shouldn't break anything.

That said, having them obviously shouldn't break the docs build. Looks like a bug in Documenter. I just upgraded Documenter to the latest version (#626). Can you see if that fixes the problem?

[bosonbaas]

Unfortunately it looks like this is still breaking, so we're still stuck with choosing between an explicit return type and docstrings.

[kris-brown]

Since it's quite common in Catlab for methods to not have return types but uncommon to have pseudo-docstrings with #, I'd opt for removing the return type.

[epatters]

Agreed. The return type is less important than the docstring.

We should also file an issue with Documenter.jl.

[bosonbaas]

I've filed an issue here (JuliaDocs/Documenter.jl#1810)

Once the return type is removed and the docs build, I'll be ready to approve this PR for merging!

[bosonbaas]

Looks great, thanks for the changes!

[bosonbaas]

The documenter issue has been closed with this PR JuliaDocs/Documenter.jl#1811, which provides workarounds for docstrings on functors. The expected behavior still doesn't work, and is dependent on the resolution of this Julia issue JuliaLang/julia#45174

[kris-brown]

important content of this is now incorporated into #777