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

Document some guarantees that should be provided by isequal #34798

Merged
merged 1 commit into from
Apr 20, 2021

Conversation

andyferris
Copy link
Member

Some documentation additions to isequal following the discussion at #34744 (comment).

@andyferris andyferris added the docs This change adds or pertains to documentation label Feb 18, 2020
base/operators.jl Outdated Show resolved Hide resolved
base/operators.jl Outdated Show resolved Hide resolved
base/operators.jl Outdated Show resolved Hide resolved
base/operators.jl Outdated Show resolved Hide resolved
@andyferris andyferris force-pushed the ajf/isequal-documentation branch 2 times, most recently from 0aec17a to e5186ad Compare February 19, 2020 02:58
@andyferris
Copy link
Member Author

I addressed the review comments, and added a examples of == and isequal for missing.

base/operators.jl Outdated Show resolved Hide resolved
base/operators.jl Outdated Show resolved Hide resolved
@andyferris andyferris force-pushed the ajf/isequal-documentation branch from e5186ad to 1189869 Compare February 19, 2020 11:59
@andyferris
Copy link
Member Author

Thanks everyone - though I feel like I had got more things wrong than I got right!

@andyferris
Copy link
Member Author

Is it OK to merge this?

Test whether `x` is less than `y`, according to a fixed total order.
`isless` is not defined on all pairs of values `(x, y)`. However, if it
Test whether `x` is less than `y`, according to a fixed total order (defined together with
[`isequal`](@ref)). `isless` is not defined on all pairs of values `(x, y)`. However, if it
Copy link
Member

@vtjnash vtjnash Feb 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this addition feels redundant to me with the first bullet point

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to sneak in a @ref, mostly. I could revert it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could add the @ref to the first bullet point instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I need to ref just isequal don't I? I can't add a link to inside a code snippet/block - I need to wrap an entire snippet. Or is the @ref thing actually smart enough to know I'm referring to just the function, like ? at the REPL?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ViralBShah
Copy link
Member

Would be nice to get this one in. What remains to be done?

Test whether `x` is less than `y`, according to a fixed total order.
`isless` is not defined on all pairs of values `(x, y)`. However, if it
Test whether `x` is less than `y`, according to a fixed total order (defined together with
[`isequal`](@ref)). `isless` is not defined on all pairs of values `(x, y)`. However, if it
is defined, it is expected to satisfy the following:
- If `isless(x, y)` is defined, then so is `isless(y, x)` and `isequal(x, y)`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe like so?

Suggested change
- If `isless(x, y)` is defined, then so is `isless(y, x)` and `isequal(x, y)`,
- If `isless(x, y)` is defined, then so is `isless(y, x)` and [`isequal(x, y)`](@ref isequal),

I tried to test whether this works as intended, but make html keeps failing:

...
[ Info: CrossReferences: building cross-references.
┌ Warning: reference for 'Inner-Constructor-Methods' could not be found in src/base/base.md.
└ @ Documenter.CrossReferences ~/Projects/julia/doc/deps/packages/Documenter/QQWIJ/src/CrossReferences.jl:104
┌ Warning: no doc found for reference '[`GetLastError`](@ref)' in src/base/c.md.
└ @ Documenter.CrossReferences ~/Projects/julia/doc/deps/packages/Documenter/QQWIJ/src/CrossReferences.jl:160
┌ Warning: no doc found for reference '[`errno`](@ref)' in src/base/c.md.
└ @ Documenter.CrossReferences ~/Projects/julia/doc/deps/packages/Documenter/QQWIJ/src/CrossReferences.jl:160
┌ Warning: no doc found for reference '[`@NamedTuple`](@ref)' in src/manual/types.md.
└ @ Documenter.CrossReferences ~/Projects/julia/doc/deps/packages/Documenter/QQWIJ/src/CrossReferences.jl:160
┌ Warning: no doc found for reference '[`StackTraces.lookup`](@ref)' in src/manual/stacktraces.md.
└ @ Documenter.CrossReferences ~/Projects/julia/doc/deps/packages/Documenter/QQWIJ/src/CrossReferences.jl:160
┌ Warning: no doc found for reference '[`mean`](@ref)' in src/manual/interfaces.md.
└ @ Documenter.CrossReferences ~/Projects/julia/doc/deps/packages/Documenter/QQWIJ/src/CrossReferences.jl:160
┌ Warning: no doc found for reference '[`std`](@ref)' in src/manual/interfaces.md.
└ @ Documenter.CrossReferences ~/Projects/julia/doc/deps/packages/Documenter/QQWIJ/src/CrossReferences.jl:160
[ Info: CheckDocument: running document checks.
[ Info: Populate: populating indices.
ERROR: LoadError: `makedocs` encountered errors. Terminating build
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] runner(::Type{Documenter.Builder.RenderDocument}, ::Documenter.Documents.Document) at /Users/jonas/Projects/julia/doc/deps/packages/Documenter/QQWIJ/src/Builder.jl:242
 [3] dispatch(::Type{Documenter.Builder.DocumentPipeline}, ::Documenter.Documents.Document) at /Users/jonas/Projects/julia/doc/deps/packages/Documenter/QQWIJ/src/Utilities/Selectors.jl:167
 [4] #2 at /Users/jonas/Projects/julia/doc/deps/packages/Documenter/QQWIJ/src/Documenter.jl:237 [inlined]
 [5] cd(::Documenter.var"##2#3"{Documenter.Documents.Document}, ::String) at ./file.jl:104
 [6] #makedocs#1 at /Users/jonas/Projects/julia/doc/deps/packages/Documenter/QQWIJ/src/Documenter.jl:236 [inlined]
 [7] (::Documenter.var"#kw##makedocs")(::NamedTuple{(:build, :modules, :clean, :doctest, :linkcheck, :linkcheck_ignore, :strict, :checkdocs, :format, :sitename, :authors, :pages),Tuple{String,Array{Module,1},Bool,Bool,Bool,Array{String,1},Bool,Symbol,Documenter.Writers.HTMLWriter.HTML,String,String,Array{Any,1}}}, ::typeof(makedocs)) at /Users/jonas/Projects/julia/doc/deps/packages/Documenter/QQWIJ/src/Documenter.jl:235
 [8] top-level scope at /Users/jonas/Projects/julia/doc/make.jl:183
 [9] include(::Module, ::String) at ./Base.jl:377
 [10] exec_options(::Base.JLOptions) at ./client.jl:297
 [11] _start() at ./client.jl:493
in expression starting at /Users/jonas/Projects/julia/doc/make.jl:183
make: *** [html] Error 1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems right. I don't know what those errors mean, since it seems like they are happening elsewhere.

@vtjnash vtjnash merged commit 1131876 into master Apr 20, 2021
@vtjnash vtjnash deleted the ajf/isequal-documentation branch April 20, 2021 18:36
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants