Skip to content

Conversation

@tkf
Copy link
Member

@tkf tkf commented May 31, 2020

Fix1 and Fix2 are already mentioned from many binary functions like

julia/base/operators.jl

Lines 950 to 959 in 7301dc6

"""
isequal(x)
Create a function that compares its argument to `x` using [`isequal`](@ref), i.e.
a function equivalent to `y -> isequal(y, x)`.
The returned function is of type `Base.Fix2{typeof(isequal)}`, which can be
used to implement specialized methods.
"""
isequal(x) = Fix2(isequal, x)

They also have clear documentation

julia/base/operators.jl

Lines 916 to 922 in 7301dc6

"""
Fix1(f, x)
A type representing a partially-applied version of the two-argument function
`f`, with the first argument fixed to the value "x". In other words,
`Fix1(f, x)` behaves similarly to `y->f(x, y)`.
"""

julia/base/operators.jl

Lines 933 to 939 in 7301dc6

"""
Fix2(f, x)
A type representing a partially-applied version of the two-argument function
`f`, with the second argument fixed to the value "x". In other words,
`Fix2(f, x)` behaves similarly to `y->f(y, x)`.
"""

I think it makes sense to list them in the documentation.

@tkf tkf added the docs This change adds or pertains to documentation label May 31, 2020
@StefanKarpinski
Copy link
Member

This makes them somewhat of an official API which definitely needs some consideration. @JeffBezanson, how do you feel about making these more official?

@tkf
Copy link
Member Author

tkf commented Jun 1, 2020

Aren't they already somewhat official? They are already mentioned from docstring of functions like isequal, <, <=, and so on:

https://docs.julialang.org/en/v1/base/base/#Base.isequal
https://docs.julialang.org/en/v1/base/math/#Base.:%3C

It says

The returned function is of type Base.Fix2{typeof(isequal)}, which can be used to implement specialized methods.

From reading this, it'd assume that Fix2 would be an official API.

@KristofferC
Copy link
Member

FWIW this is why I don't like documenting internals with docstrings. So easy for someone to just add a reference to it and all of a sudden it is public API.

@tkf
Copy link
Member Author

tkf commented Jun 5, 2020

documenting internals with docstrings

If you mean Base.Fix2 docstring, this is not what is happening here. Base.Fix2{typeof(isequal)} is mentioned in the documentation of isequal, not Fix2.

I think the docstring of isequal clearly indicates that dispatching on Base.Fix2{typeof(isequal)} is a public API:

The returned function is of type Base.Fix2{typeof(isequal)}, which can be used to implement specialized methods.

Of course, if the intention was to explain the idea behind the implementation (which is OK to do in a public docstring), we can clarify that. But this is a minor change.

(But I think exposing Fix1/Fix2 as public API is very useful.)

@KristofferC
Copy link
Member

If you mean Base.Fix2 docstring, this is not what is happening here. Base.Fix2{typeof(isequal)} is mentioned in the documentation of isequal, not Fix2.

I know, this was just a (slightly tangential) remark that it becomes hard for people to know what is public API with internals having docstring and it is easy to accidentally make something public by e.g. referencing it from something that actually is shown in the docs.

@tkf
Copy link
Member Author

tkf commented Jun 5, 2020

Well, that's my whole motivation for clearly documenting what public API means #35715. I think it's a big loss if we discourage documenting/doctesting internals just for avoiding API missuses. Mentioning the definition of public API in the documentation may not be enough. But I think tooling-based fixes would be better than discouraging internal documentation.

Meanwhile, maybe we can use some kind of marker when adding docstrings to internal:

    Fix1(2, x) 

**INTERNAL**
A type representing a partially-applied version of the two-argument function
...

@clarkevans
Copy link
Member

clarkevans commented Jul 16, 2020

I hope that Base.Fix1 and Base.Fix2 are not simply internal details. I'd like to be able to use the documented interface for query macros. Let me provide a tangible use case. Currently, our users use the two-argument version of contains lifted to queries...

julia> using DataKnots

julia> @query unitknot "Hello World".contains(it, "Hello")
┼──────┼
│ true │

A fluent-style, single argument version would be really nice. However, if one attempts to use that, here's what we currently get....

julia> @query unitknot "Hello World".contains("Hello")
┼────────────────────────────────────────────────────┼
│ Fix2{typeof(contains),String}(contains, \"Hello\") │

If we could reliably know that Fix2 represents a curried function, we might automatically apply this function on the input to get the expected output. This would provide a usability improvement.

julia> @query unitknot "Hello World".contains("Hello")
┼──────┼
│ true │

Is it appropriate to rely upon Fix2 as being a public interface in this way? Moreover, for our own curried functions, could we use Fix1 and Fix2 directly?

Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

I don't see us changing these, and we already use them for enhanced dispatch internally. I think it is reasonable to allow other packages to do the same. I'll merge if no objections soon.

@goretkin
Copy link
Contributor

I don't see us changing these, and we already use them for enhanced dispatch internally. I think it is reasonable to allow other packages to do the same. I'll merge if no objections soon.

I worry that Fix1/Fix2 is a limited (albeit important) special case of something more general, and that it might be premature for its use to become widespread.

On the other hand, I think it may be possible to generalize it, and introduce Fix1/Fix2 as aliases, as I've done here:
https://github.com/goretkin/FixArgs.jl/blob/ed66e762481dcebcce4400a6a7fa1b8061eed52f/src/ergonomics.jl#L16-L24

@JeffBezanson JeffBezanson merged commit 00b1807 into JuliaLang:master May 19, 2021
@goretkin
Copy link
Contributor

Could someone comment on whether this would be breaking:

On the other hand, I think it may be possible to generalize it, and introduce Fix1/Fix2 as aliases, as I've done here:
https://github.com/goretkin/FixArgs.jl/blob/ed66e762481dcebcce4400a6a7fa1b8061eed52f/src/ergonomics.jl#L16-L24

shirodkara pushed a commit to shirodkara/julia that referenced this pull request Jun 9, 2021
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
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.

8 participants