typelevel / cats

Lightweight, modular, and extensible library for functional programming.
https://typelevel.org/cats/
Other
5.25k stars 1.21k forks source link

Renaming Cartesian to Semigroupal #1931

Closed edmundnoble closed 7 years ago

edmundnoble commented 7 years ago

Cartesian is our name for an associative def product[A, B](fa: F[A], fb: F[B]): F[(A, B)]. I find this name very confusing given that cartesian categories are not the same thing as monoidal and semigroupal functors, and there's no reason to associate the action of a semigroupal functor with a cartesian product or zipping seeing as they have much more meaning semantically and aren't actually provided by most instances of Cartesian as it stands. I can see some doubt in the previous PR, https://github.com/typelevel/cats/pull/795, as to whether or not this would be called a "semigroupal" functor. As far as I can tell it is; google "semigroupal functor" for examples, one of which is https://www.researchgate.net/publication/2100971_Abelian_Categories_of_Modules_over_a_Lax_Monoidal_Functor which suggests a definition by analogy to monoidal functors.

With all of that in mind, I'm calling for us to rename Cartesian to Semigroupal before 1.0.0-RC1 is out. I don't know if there's a convincing case to be made for Monoidal to be added.

Attribution: @ekmett at scalaworld told me in passing that the name was incorrect when I mentioned it.

johnynek commented 7 years ago

I think we should only consider this if we retain an alias to the name Cartesian. In fact Cartesian's product method is one of the most useful methods in cats (producing a typeclass for (A, B) when you have it for A and B in the every common case that F[_] is cartesian).

arosien commented 7 years ago

From a teaching FP perspective, Cartesian is confusing enough, compared to its scalaz Zip origins. Semigroupal would be even more confusing. I'm happy with technically correct aliases, however.

edmundnoble commented 7 years ago

@johnynek I agree that it's useful, but I think using an alias entirely removes the benefit of renaming and significantly increases the potential for confusion.

@arosien In my opinion Zip is significantly more confusing. It's fine for pedagogy, at first, but it's not what this actually is. What makes something "zipping" is significantly more than what makes something an associative product of functors.

johnynek commented 7 years ago

so, have seen numerous shops stay on old versions of libraries because the breakage is a pain no one wants to fix.

An incompatible change that takes N minutes to fix should be multiplied by the number of downloads to think about the order of the cost we are imposing.

I am pretty negative on us doing it just to polish when an alias costs us nothing. I think this is hostile to industrial users, and I am trying to foster industrial usage.

edmundnoble commented 7 years ago

I think the cost of an alias is exactly the cost of keeping it as it is, which implies there are only two real choices here: rename with no alias, or don't do it at all.

tpolecat commented 7 years ago

What if we add a deprecated alias?

scala> @deprecated("Use Int!", "now") type X = Int
defined type alias X

scala> val z: X = 42
<console>:12: warning: type X is deprecated (since now): Use Int!
       val z: X = 42
              ^
z: X = 42
edmundnoble commented 7 years ago

I'm fine with that. That doesn't seem to solve the issue that users still need to migrate. I don't really think migration will take >2 minutes, however.

adelbertc commented 7 years ago

When I added Cartesian I was super tempted to call it Semigroupal if only because it would match up with the Monoidal and Monoid parallel. Good to hear the name is technically correct. 👍 from me.

johnynek commented 7 years ago

Just imagine doing these changes in a 6 million line code base. Not all the uses are yours. So you have to get teams across the org to sign off on the change. They all have to do it atomically because you can’t bump cats for the whole repo until the whole change is green.

And each time we make a change we add to the complexity of this PR.

This is not a hypothetical. I get that not everyone has had this experience, but I think it should be easy enough to imagine once explained. Multiple that by each large industrial scala user.

2 minutes is absurd.

LukaJCB commented 7 years ago

I share @johnynek's concerns. However, I also think that when we combine a deprecated type alias with a scalafixscript, this really shouldn't be too much of a problem. :)

In fact, maybe we should make it a rule for breaking changes that can be fixed by scalafix to also provide a scalafix script.

julienrf commented 7 years ago

I confirm that for things like renaming a symbol scalafix works well.

kailuowang commented 7 years ago

I prefer to do the rename and provide a deprecated type alias, as well as a scalafix if we have time.

the cost of an alias is exactly the cost of keeping it as it is

I am not sure if that's true. The current name is confusing, but the confusion already happened. Whoever uses it now is already over with that confusion, forcing them to rename it would just be extra cost. Renaming it will successfully avoid the confusion to future users coming to Cats. Having a deprecated alias (and documented) probably won't confuse them.

Also, mentions of the old name already exist on the internet (blogs, books, articles, talks), keeping an alias may actually help when people trying to find it.

edmundnoble commented 7 years ago

Yeah I agree that a deprecated alias is a good idea. That comment was directed toward non-deprecated aliases.

tpolecat commented 7 years ago

I wish there were a way to search GH to see how many uses there are in the observable wild. I would expect there to be few explicit instances, and most of the uses to be via .mapN.

adelbertc commented 7 years ago

On a related note if we're going to call this Semigroupal I feel we should also add Monoidal somehow.. which then enters a discussion of how that plays with Applicative.

edmundnoble commented 7 years ago

I don't think we need Monoidal unless there's a convincing case that we do.

kailuowang commented 7 years ago

Looks like at least we can proceed with a PR that does the rename and provide a deprecated alias.