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

Add ASCII alias compose of #33573

Closed
wants to merge 4 commits into from
Closed

Conversation

tkf
Copy link
Member

@tkf tkf commented Oct 16, 2019

There are quite a few cases where composition becomes/can be used as fundamental API in Julia:

However, the composition operator is only exposed by a non-ASCII name. This is not an ideal situation because the common practice in Julia is to provide ASCII-based API even when non-ASCII names are the primary API. An easy solution implemented in this PR is to define const compose = ∘ in Base.

@@ -851,10 +852,14 @@ julia> fs = [

julia> ∘(fs...)(3)
3.0

julia> ∘(fs...) === compose(fs...)
Copy link
Member

Choose a reason for hiding this comment

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

I admit to be surprised by this property: is it documented or an implementation detail of functions returning anonymous functions?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'm not sure that's the best example. Maybe apply the functions to a set of points and show that they always give the same results instead?

Copy link
Member

Choose a reason for hiding this comment

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

For union we don't even bother to use the unicode equivalent in the docstring. I think just repeating the previous example but with compose would be enough (compose(fs...)(3) ==> 3.0).

Copy link
Member Author

Choose a reason for hiding this comment

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

@rfourquet Thanks for the suggestion. It makes sense. I updated the docstring.

NEWS.md Outdated Show resolved Hide resolved
@JeffBezanson
Copy link
Member

https://github.com/GiovineItalia/Compose.jl/blob/2b561b0f53a81cbf9a4001d721d29880d6e50ebc/src/Compose.jl#L18 ...
Maybe we should call it fcompose?

@tkf
Copy link
Member Author

tkf commented Oct 16, 2019

https://github.com/GiovineItalia/Compose.jl/blob/2b561b0f53a81cbf9a4001d721d29880d6e50ebc/src/Compose.jl#L18 ...
Maybe we should call it fcompose?

I think this is a similar situation as the addition of eachrow/eachcol which conflicted with DataFrames etc. #29749. It was decided that it was OK to introduce new functions even when conflicting with registered packages. So I suppose using compose here also makes sense?

@Keno
Copy link
Member

Keno commented Oct 16, 2019

Maybe we should call it fcompose?

👍, I was gonna make the same comment and propose the exact same name :)

@tkf
Copy link
Member Author

tkf commented Oct 16, 2019

I think fcompose implies that it's about the composition of functions. It would be unfortunate since can be and is used for composition of other abstract concepts as I listed in the OP. It is true that those examples can be implemented in terms of concrete function composition. But I think it would be nice to keep it abstract so that it is natural to use in wider range of situations.

@Keno
Copy link
Member

Keno commented Oct 17, 2019

For those examples, does the composition follow the functional composition law f ∘ g = f(g(x))? If so then I think using functional composition as the term of art here is still appropriate. If not, I'm wondering whether they should really be part of the same generic function.

@tkf
Copy link
Member Author

tkf commented Oct 17, 2019

All of the examples above can be implemented as functions, and if done so, would satisfy (f ∘ g)(x) = f(g(x)). But it may not be the most useful interface. For example, Setfield.Lens provides interfaces set(obj, lens::Lens, val) and get(obj, lens::Lens) with compositions done via ∘(::Lens, ::Lens). Compare this to function-based definition in Haskell:

type Lens s a = forall f. Functor f => (a -> f a) -> (s -> f s)

A lens maps a -> f a to s -> f s. So yes, (lens1 ∘ lens2)(x) = lens1(lens2(x)) holds, but most of the users would never invoke this directly and thus it's rather impractical to expose this API.

Given that is a common notation for composition in category theory, I think it is appropriate to use for composition operation of things (arrows) as long as they have the identity and the operation is associative. As using a shared verb for similar operations is considered a good practice in Julia, isn't it natural to use for composition in general?

@Keno
Copy link
Member

Keno commented Oct 17, 2019

The point we're making is that compose is too general a word to refer solely to composition of morphisms. E.g. what if my AI system wants to music = compose("gospel", "death metal") or as pointed out above, composition of graphics in Gadfly. Given that the term is overloaded, it seems wrong to steal it, especially for an alias of a unicode operator. Users who really want it pretty can always use that. I don't particularly care whether it's fcompose or something better.

@Keno Keno added the triage This should be discussed on a triage call label Oct 17, 2019
@Keno
Copy link
Member

Keno commented Oct 17, 2019

I think composite (as in return f ∘ g the composite of f and g [1]) may work, though we should keep thinking about alternatives and see what we like best.

[1] https://ncatlab.org/nlab/show/composition

@tkf
Copy link
Member Author

tkf commented Oct 17, 2019

The point we're making is that compose is too general a word to refer solely to composition of morphisms.

I think this argument can applied many functions in Base. map can be a factory function of associative data structures instead of the higher-order function. join can mean an operation on tables instead of the operation on strings [*]. At some point you need to cherry pick one meaning if a short word you want to use is overloaded. I think choosing a concept that is applicable to many situations is a good guideline.

I listed the examples in OP to motivate that there are distinct cases where using compose as composition of morphisms is appealing. I don't think composition of music or graphics has a wider application that can be described with a "common verb."

[*] I know DataFrames.jl uses Base.join anyway and it may be a good decision in terms of practicality. But IIUC what C++ does with >> is not a good practice in Julia. This is why I'd like to avoid using fcompose for composition of non-functions if possible.

@Keno
Copy link
Member

Keno commented Oct 17, 2019

Another option would be to just call the alias circ, since you type it using \circ. Just adding to the list of options. Clearly triage needs to come to a resolution here.

@tkf
Copy link
Member Author

tkf commented Oct 17, 2019

Just as a source of inspiration, quoting Function composition - Wikipedia:

The notation g ∘ f is read as "g circle f", "g round f", "g about f", "g composed with f", "g after f", "g following f", "g of f", or "g on f".

Most of them are using pretty generic word, though.

@andyferris
Copy link
Member

andyferris commented Oct 17, 2019

The original PR had compose defined... and I still think that would be the best word out of the above suggestions.

It seems ok for Base to choose one of the meanings of a preexisting verb that exists in the ecosystem. (CoordinateTransformations.jl has exported compose with this meaning since I think before Base had the composition operator at all).

@StefanKarpinski
Copy link
Member

Just to spell this out: if Base starts exporting compose then any code that does using Gadfly and later calls compose(...) expecting to call Gadfly.compose will start to fail. That's not a show-stopper, but it's a consequence that everyone in this discussion should be aware of.

@nalimilan
Copy link
Member

My two cents: it's not really relevant that Gadfly breaks as long as we're OK with it doing import Base: compose once this PR is merged. We did the same with eachcol in DataFrames and it went fine, we have plenty of time to make a Gadfly release before 1.4 is released.

What matters is what's the best name for Julia.

@dpsanders
Copy link
Contributor

I always think of this as the "composition" operator, so why not call it composition?

@kmsquire
Copy link
Member

kmsquire commented Oct 19, 2019

My two cents: it's not really relevant that Gadfly breaks as long as we're OK with it doing import Base: compose once this PR is merged.

Since we're following semver, this seems like a breaking change. If we call this compose and release this with Julia version 1.4, then suddenly some code which ran perfectly fine on 1.3 will no longer run without changes on 1.4.

My vote would be to call it compose, but wait until 2.0 to include it.

@andyferris
Copy link
Member

@kmsquire I’ve often wondered about semver and Julia. Technically any API addition (new exported symbol or new method on exported generic function) could cause ambiguity and code to break. That leaves the question of what the purpose of semver minor tags is in Julia. Pragmatically in my packages I’ve used minor to indicate new API (including exports) and major for changes or removal of existing APIs.

@tkf
Copy link
Member Author

tkf commented Oct 20, 2019

SemVer specifies that Software using Semantic Versioning MUST declare a public API. Is it declared that Set(names(Base)) is non-increasing? OK, I think strictly following SemVer is not really practical for software with huge API surface like julia. But I thought that's what minor change is for? Also, as mentioned above a few times this already happened for eachrow/eachcol #29749.

As a side note, I've been also thinking to suggest adding "Always prefer explicit import (using M: x)" in the style guide, to avoid situations like this. Note that we can detect possible breakages like this in registered packages but not in private packages.

@Keno
Copy link
Member

Keno commented Oct 20, 2019

But I thought that's what minor change is for?

Yes, this is correct. Adding exports is allowed in minor versions. Of course, we also have a soft promise not to actually break existing packages (i.e. PkgEval should be clean), so Compse/Gadfly may want to choose a new name if we do go with compose.

@tkf
Copy link
Member Author

tkf commented Oct 20, 2019

I always think of this as the "composition" operator, so why not call it composition?

@dpsanders I thought it'd be natural for an operator to be a verb rather than a noun. But maybe not the case? Looking at Base and stdlib, they are all abbreviations like div for /, mul! for *, etc. and it's not clear if they are verbs or nouns.

Reflecting this trend would mean that comp for ? It seems Clojure uses it https://clojuredocs.org/clojure.core/comp. But it's dangerously close to cmp...

@JeffBezanson
Copy link
Member

as long as we're OK with it doing import Base: compose once this PR is merged

It seems to me gadfly's sense of compose is very different from this one.

@StefanKarpinski
Copy link
Member

Since we're following semver, this seems like a breaking change.

We've always been pretty clear on the fact that if you rely on implicit using to get names then your code is not future-proof. If you explicitly import names you rely on (as is recommended) then your code will not break. We should probably make a document outlining this more officially, but otherwise it's impossible to change anything in the language and we might as well all go home.

@andyferris
Copy link
Member

I know this issue isn’t exactly about changing Gadfly/Compose but I wonder if it is useful to note that there exists a verb composite that matches the semantic of composing/compositing graphics (potentially leaving the mathematical/functional semantic of compose for Base).

@andyferris
Copy link
Member

@StefanKarpinski On that note I’ve always felt that I’d love a tool that intelligently converts using Package statements into explicit usings/imports (and perhaps even have the package evaluator warn about this stuff when registering in General).

@JeffBezanson
Copy link
Member

I feel like the conciseness of infix is sort of the whole point of having this as a function. Otherwise you might as well just write x->f(g(x)).

@JeffBezanson
Copy link
Member

Triage is slightly against this, but we are ok with calling it fcompose. Conflicting with Compose.jl is a bit much to ask just to add an alias.

@tkf
Copy link
Member Author

tkf commented Nov 1, 2019

The main theme of this PR is to provide a canonical verb for composition of morphisms. Was it discussed in the triage? Ideally, I would like to know triage's answer to these questions:

  1. How much is it worth to have a canonical verb for composition of morphisms in the Julia ecosystem?

    • If so, should be used?
    • Is it important enough for Base to provide an ASCII alias?
  2. How does it compare to the value of maintain the compatibility of the code relying on implicit imports?

Of course, my opinion on the first point is that it's quite large since there are already at least three independent packages that can use this API (see the OP). For the second point, I note that implicit import is known to be not forward-compatible across minor versions #33573 (comment).


Having said that, I note a counter argument, a counter-counter argument, and a counter-counter-counter argument:

A counter argument may be that it is possible for each package to define an alias const compose = ∘ and export it. This would not cause name conflict warning as long as all packages agreeing on the name. However, it would then become strange to use compose from one package with morphisms on another package if they don't relate each other. For example:

using Setfield: @lens, compose
using Transducers: Map, Filter

compose((@lens _.a), (@lens _.b))  # this looks ok
compose(Map(f), Filter(g))         # why is `compose` usable with transducers?

A better way to do this is to define a package

module FunctionalBase

"""
    compose

An alias of `∘`.
"""
const compose = 

end

so that the above example with

using FunctionalBase: compose
using Setfield: @lens
using Transducers: Map, Filter

makes sense. Another benefit is that the docstring shown via ?compose would be consistent (i.e., does not depend on import order).


tl;dr As I think per-package alias or an alias in a shared base package would work reasonably well, I'm going to close this PR.

Thanks a lot for the discussion!

@tkf tkf closed this Nov 1, 2019
@c42f
Copy link
Member

c42f commented Nov 1, 2019

I feel like the conciseness of infix is sort of the whole point of having this as a function. Otherwise you might as well just write x->f(g(x)).

I think this makes sense when is used for lazy composition of opaque functions f and g. But sometimes f and g are function objects with enough structure that they can be eagerly combined together for improved efficiency.

For example, in CoordinateTransformations f and g are often linear or affine transformations on a vector space and we define compose as an eager operation where possible.

@tkf
Copy link
Member Author

tkf commented Nov 1, 2019

The adjoint conversion of iterator transforms to transducers I demonstrated in #33526 is also another example where you can do more when can "see" more structure. (Although in #33526 I avoid using to minimize the footprint of the PR.)

@andyferris
Copy link
Member

Agreed - seeing the structure of the computations can be super useful for a variety of optimizations.

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.

10 participants