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

Renaming Cartesian to Semigroupal #1931

Closed
edmundnoble opened this issue Sep 26, 2017 · 17 comments · Fixed by #1961
Closed

Renaming Cartesian to Semigroupal #1931

edmundnoble opened this issue Sep 26, 2017 · 17 comments · Fixed by #1961
Assignees

Comments

@edmundnoble
Copy link
Contributor

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, #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
Copy link
Contributor

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
Copy link

arosien commented Sep 26, 2017

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
Copy link
Contributor Author

@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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Member

tpolecat commented Sep 27, 2017

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

LukaJCB commented Sep 28, 2017

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
Copy link
Contributor

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

@kailuowang
Copy link
Contributor

kailuowang commented Sep 28, 2017

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
Copy link
Contributor Author

edmundnoble commented Sep 28, 2017

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

@tpolecat
Copy link
Member

tpolecat commented Sep 28, 2017

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
Copy link
Contributor

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
Copy link
Contributor Author

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

@kailuowang
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants