-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document atoms #610
Document atoms #610
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #610 +/- ##
=======================================
Coverage 98.21% 98.21%
=======================================
Files 89 87 -2
Lines 5198 5198
=======================================
Hits 5105 5105
Misses 93 93 ☔ View full report in Codecov by Sentry. |
the current docs are kind of centered around the constructor functions rather than the structs, since usually users should use the former. I think though it definitely is nice to document the actual primitives that are available. I think maybe it could be most useful to keep the functions as the first thing the users see/interact with, and add docstrings to them, and then have the atoms as more advanced docs. |
Yeah... I've realized this. I just wondered about adding docstrings for I don't really have a good solution yet. |
There's also: https://jump.dev/Convex.jl/previews/PR610/manual/operations/ But it doesn't have every atom or reformulation (e.g., I also wonder about how important the DCP rules are. I don't think many people are computing the rules in their head from the docs? They probably just try it to see if it works. |
What are thoughts on https://jump.dev/Convex.jl/previews/PR610/atoms/ Too verbose? What parts are useful? |
I like it! IMO not too verbose. In fact I think the reformulations could be more verbose. I think the examples are quite useful, and I like the mini table too about convexity properties. I find them useful at least, and I think when folks are debugging stuff it comes in handy. |
Yes, if we're not focusing on atoms, then it makes sense to have these identical. I even wonder if it makes sense to move all of the |
true, and we have done that in the past (though often the other way, deleting slow or buggy atoms in favor of reformulations) |
77f8b32
to
c29661c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this so I think we should merge before releasing v0.16. I've added it in the milestone
I don't know if this needs to block the release |
It does not, you can remove it from the milestone if this PR is not ready to merge |
@blegat thoughts on keeping the operators in a separate file or keep with atom structs? |
Here's a script to rebuild the doclist function build_atoms_md(input_filename, filename)
data = read(input_filename, String);
reform = joinpath(dirname(input_filename), "reformulations")
for file in readdir(reform; join = true)
data *= read(file, String)
end
header = """# Atoms
Convex.jl supports the following functions. These functions may be composed
according to the [DCP](http://dcp.stanford.edu) composition rules to form new
convex, concave, or affine expressions.
"""
open(filename, "w") do io
return print(io, header)
end
block_fn = r"\"\"\"\nfunction (.+?\(.+?\))"ms
inline_fn = r"\"\"\"\n([a-zA-Z0-9\_\.\:\+\-\*\/\^]+\(.+?\)) = "m
function fn_name(x)
x = replace(x, "Base." => "", "LinearAlgebra." => "", ":" => "")
return String(x[1:(only(findfirst("(", x))-1)])
end
args = Dict{String,Vector{String}}()
block_fns = collect(eachmatch(block_fn, data))
inline_fns = collect(eachmatch(inline_fn, data))
for m in vcat(block_fns, inline_fns)
is_public = occursin("Base.", m[1]) ||
occursin("LinearAlgebra.", m[1]) ||
Symbol(fn_name(m[1])) in names(Convex)
if !is_public
continue # Not exported
end
signature = replace(
m[1],
r"[a-zA-Z]+?::" => "::",
"LinearAlgebra." => "Convex.",
"AbstractExpr" => "Convex.AbstractExpr",
"Value" => "Convex.Value",
"Constant" => "Convex.Constant",
)
if !(startswith(signature, "Base.") || startswith(signature, "Convex."))
signature = "Convex.$signature"
end
push!(get!(args, fn_name(m[1]), String[]), signature)
end
open(filename, "a") do io
for name in sort(collect(keys(args)))
signature = join(sort(args[name]), "\n")
println(
io,
"""
## `$name`
```@docs
$signature
```
""",
)
end
return
end
return
end
build_atoms_md("src/supported_operations.jl", "docs/src/manual/atoms.md") |
I like it in a separate file |
So what do we think about this? |
docs/src/manual/atoms.md
Outdated
@@ -0,0 +1,397 @@ | |||
# Atoms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe "Operations"?
docs/src/manual/atoms.md
Outdated
Convex.jl supports the following functions. These functions may be composed | ||
according to the [DCP](http://dcp.stanford.edu) composition rules to form new | ||
convex, concave, or affine expressions. | ||
## `*` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved this to the API reference, so I think I'm going to merge, unless there are other comments. |
Just exploring a few options:
https://jump.dev/Convex.jl/previews/PR610/reference/atoms/#Supported-operations
I still have these to go: