improve documentation, add doctests, fix bugs found on the way #11
Conversation
|
|
||
| ``` | ||
| """ | ||
| explain |
There was a problem hiding this comment.
This explain kinda comes out of nowhere, eh?
src/core.jl
Outdated
| Looks for a match for `query` in the text. Returns either `nothing` | ||
| if no match is found, or a [`QueryMatch`](@ref) object. | ||
| """ | ||
| Base.match(query::AbstractQuery, text::Union{Document,Corpus}) |
There was a problem hiding this comment.
Maybe I'm missing something obvious, but where's the function definition?
There was a problem hiding this comment.
Yeah, so in all these I'm on-purpose not attaching the docstring to the specific methods, because the existing methods are more narrow than where I want the docstring to apply. E.g. here I want the docstring to apply to any method of Base.match that fits that signature (instead of sticking it on just one of the match methods, or copying it by hand to all of them). For the explain one, I want it to apply to any method of explain.
Why does it matter? if for example you do ? + then you get all the docstrings for any method of +. But if you do ? +(1,1), you get just 1 docstring, the most specific one that applies to those arguments.
src/core.jl
Outdated
| Looks for all matches for `query` in the text. Returns either a | ||
| `Vector` `QueryMatch` objects corresponding to all of the matches found. | ||
| """ | ||
| match_all(query::AbstractQuery, text::Union{Document,Corpus}) |
|
preview is at https://beacon-biosignals.github.io/KeywordSearch.jl/previews/PR11/, which you can get from clicking the "details" button in the "documenter/deploy" check |
|
Marking as draft since I want to do some more doc changes |
|
Ok, I made a bunch of changes, which I tried to group into clear commits. I found two bugs when improving the docs:
I decided to embrace doctests, in particular for testing printing, because it's easy to update them if the printing changes (see I think the codecov drop is spurious; the lines it says that are not covered are line 31-34 in printing.jl (https://app.codecov.io/gh/beacon-biosignals/KeywordSearch.jl/compare/11/changes#D1L31), but those are covered by the doctests for the show method for |
ericphanson
changed the title
ericphanson
force-pushed
the
eph/betterdocs
branch
from
4a726f9 to
ded99e8
Compare
| @testset "Doctests" begin | ||
| DocMeta.setdocmeta!(KeywordSearch, :DocTestSetup, :(using KeywordSearch); | ||
| recursive=true) | ||
| Documenter.doctest(KeywordSearch) |
|
I rebased after merging the others and fixed an issue with the printing. Not totally sure what happened, but I think doctests are very sensitive to whitespace, i.e. you need to start with the I also learned from https://discourse.julialang.org/t/show-and-showcompact-on-custom-types/8493/4 that 3-argument show is preferred over checking |
I realized that after #8 some docstrings I didn't want to include (e.g.
_findmin, which hopefully will be gone soon, ref #9) were there, and others didn't exist (e.g.matchandexplain). So here I've tried to fix that.