cleanup Flux.Losses documentation #1930
Conversation
docs/make.jl
Outdated
| @@ -1,6 +1,11 @@ | |||
| using Documenter, Flux, NNlib, Functors, MLUtils | |||
|
|
|||
| DocMeta.setdocmeta!(Flux, :DocTestSetup, :(using Flux); recursive = true) | |||
| DocMeta.setdocmeta!(Flux.Losses, :DocTestSetup, :(using Flux.Losses); recursive = true) | |||
There was a problem hiding this comment.
it seems this line is not doing its job and docs CI is failing because of this
There was a problem hiding this comment.
I believe these lines are useless since makedocs have the tests disabled. In the GH CI.yml, there is a separate version of these lines, and that's the one that matters.
There was a problem hiding this comment.
turns out that there is an additional problem, DocMeta.setdocmeta! doesn't accept the DocTestFilters that I also needed, so I had to revert this change entirely
| julia> Flux.mse(y_model, y_true) | ||
| julia> mse(y_model, y_true) | ||
| 0.010000000000000018 |
There was a problem hiding this comment.
Why change these?
Elsewhere in Flux, all (ideally!) non-exported functions are qualified in doctests. It seems important to know you have to, seems bad to write examples which won't work at the repl.
Maybe the logic is that we are inside the module Losses, but (IMO) expecting confused users to be aware of such internal details is asking a lot. Better the Flux examples all work after loading Flux.
There was a problem hiding this comment.
added using Flux.Losses: mse statements
Codecov Report
@@ Coverage Diff @@
## master #1930 +/- ##
=======================================
Coverage 86.65% 86.65%
=======================================
Files 18 18
Lines 1416 1416
=======================================
Hits 1227 1227
Misses 189 189
Continue to review full report at Codecov.
|
|
Thoughts on rebasing this since we've had some QoL changes to the docs build recently? cc @Saransh-cpp |
|
Yes, rebasing should work! Additionally, I think these lines - Flux.jl/src/losses/functions.jl Lines 1 to 4 in 8bc0c35 are redundant, and can be removed. Rebase should also get the DocBot to comment here. Currently, the updated docs can be viewed through the |
afterthought of #1928