Skip to content
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

docs 1.0 upgrade #137

Merged
merged 1 commit into from
Sep 16, 2023
Merged

docs 1.0 upgrade #137

merged 1 commit into from
Sep 16, 2023

Conversation

ArnoStrouwen
Copy link
Member

No description provided.

@ChrisRackauckas ChrisRackauckas merged commit f317b06 into SciML:master Sep 16, 2023
15 of 17 checks passed
@ArnoStrouwen ArnoStrouwen deleted the docs1.0 branch September 16, 2023 20:55
@ArnoStrouwen
Copy link
Member Author

@mortenpi is it expected behavior that Documenter complains about LinearAlgebra docstrings, while that module is not included in the modules keyword?
https://github.com/SciML/ExponentialUtilities.jl/actions/runs/6209285369/job/16856334243#step:5:28
https://github.com/SciML/ExponentialUtilities.jl/blob/master/docs/make.jl#L10

@mortenpi
Copy link

mortenpi commented Sep 17, 2023

You're overloading that function and attaching a docstring to it here:

"""
stegr!(α, β, sw)
Diagonalize the real-symmetric tridiagonal matrix with `α` on the
diagonal and `β` on the super-/subdiagonal, using the workspaces
allocated in `sw`.
"""
function stegr!::AbstractVector{T}, β::AbstractVector{T}, sw::StegrWork{T}) where {T}
# @assert length(sw.dv) >= length(α)
# @assert length(sw.ev) >= length(β)
copyto!(sw.dv, α)
copyto!(sw.ev, β)
stegr!(BlasInt(length(α)), sw)
end

So yes, it's expected. It's searching for docstrings (not functions etc) in your modules.

Edit: I think we can do a better job with the error here though: JuliaDocs/Documenter.jl#2265

@ChrisRackauckas
Copy link
Member

Why is it not just the public functions? JuliaLang/julia#50105 defines the public semantics, why wouldn't Documenter follow them?

@mortenpi
Copy link

I mean.. that PR got merged like 2 days ago, and doesn't even exist in a Julia release. That said, yes, I do think we want to update Documenter's logic here eventually (JuliaDocs/Documenter.jl#600 (comment)).

@ChrisRackauckas
Copy link
Member

Well you were saying you were going to throw a better error in the future. I was just saying that if anything should happen in the future, this case shouldn't error unless it's a public function.

@mortenpi
Copy link

mortenpi commented Sep 17, 2023

Ah. Well, you can still run into the error if you're legitimately extending a function from somewhere else. The confusion here, from my perspective, is that it looks like Documenter is complaining about someone else's docstring. But it's actually complaining about one of yours, which you do want to include in the manual (probably; if it's public stuff that is). So making this error better would be useful regardless.

That said.. LAPACK.stegr! is a public function though, no...? Or if it isn't, then why is ExponentialUtilities mucking with it? Or.. if it's adding like a private method, then the public keyword won't really help here, because you're running into the private method edge case that the new keyword doesn't really handle?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants