Binary-compatibly move typelevel/algebra into cats repo #3918

armanbilge · 2021-06-04T21:13:36Z

Following directly from #3877 (comment).

Re-played commits from #3877:

Brings in Scala 3 final

Addresses typelevel/algebra#246 (comment)

armanbilge · 2021-06-05T02:35:59Z

Ok everyone, good news: I was able to rework @denisrosset's proposals typelevel/algebra#246 and typelevel/algebra#247 as well as typelevel/algebra#108 so that they are binary compatible 😅

Unfortunately I had issues with 44eb36c which tries to reduce potential for conflicts when using import algebra.instances.all._. It seems that marking as private[algebra] a trait method that is extended by a (package) object causes the static-forwarder for that method to disappear completely.

Still haven't brought in any of the significant changes to laws yet, but since it seems breaking bin-compat there could be okay they are less of a concern.

Finishes merging typelevel/algebra#248

armanbilge · 2021-06-19T22:31:22Z

@denisrosset @larsrh @johnynek @mpilquist Ready for review. Hopefully we can wrap this up? :)

I decided not to make any binary-breaking changes to laws, doing so doesn't feel right under semantic versioning. Furthermore, I decided to keep everything in both core/laws source-compatible and added @deprecation warnings instead.

In the end I was able to replay most of the changes from the original PR. The glaring omissions are:

Currently, both cats and algebra instances inherit from cats-kernel instances. So importing both could cause conflicts/problems, possibly? It doesn't seem possible to fix this in a binary-compatible way.
The organization/naming scheme of algebra-laws is not very Cats-like (separation of laws and tests, etc.). Since breaking bincompat for laws seemed a possibility, I could still do this, but this would also mean breaking semantic versioning rules.

Thanks all!

johnynek · 2021-06-19T22:36:44Z

I'll look soon. Note: removing "implicit" is actually binary (although not source) compatible.

Since source incompatibilities are always discovered by compilation but binary are often discovered via runtime crashes in transitive dependencies, I would consider removing implicits that conflict to be acceptable.

armanbilge · 2021-06-19T22:40:34Z

Note: removing "implicit" is actually binary (although not source) compatible.

Thanks, I am aware of this! See my comment above #3918 (comment) about the problems I had with this. I don't think removing implicit would help at the source-level without also marking as private[algebra], because implicits and non-implicits alike get imported with the usual wildcard imports. So conflicting names would still be a problem.

johnynek

I'm mostly good. I had a couple of minor comments.

johnynek · 2021-06-30T19:03:55Z

algebra-core/src/main/scala/algebra/Priority.scala

+ * This type can be useful for problems where multiple algorithms can
+ * be used, depending on the type classes available.
+ */
+@deprecated("No replacement", since = "2.7.0")


why is this deprecated? It seems it is not deprecated in algebra?

The motivation is that some functions can be written that e.g. use either a Group or a Monoid. If you have a Group, it can be more efficient, or else you can fall back to Monoid (at a constant multiplicative cost).

Note: we could imagine pushing this onto Either[A, B] in the scala standard library (prefer Right, fallback to Left). Similarly with Option[A] and tuples, but I think the standard library currently does not allow this.

I was just replaying 7ff1809 which was suggested on the original PR since it's not used in algebra itself. My bad, now I see its intrinsic value. I'll bring it back.

Btw I did see your plans to submit it to Scala standard library in typelevel/algebra#67!

johnynek · 2021-06-30T19:04:12Z

algebra-core/src/main/scala/algebra/Priority.scala

+    fold[Option[F]](_ => None)(f => Some(f))
+}
+
+@deprecated("No replacement", since = "2.7.0")


can we remove this?

johnynek · 2021-06-30T19:11:59Z

algebra-laws/jvm/src/main/scala/algebra/laws/platform/Platform.scala

@@ -0,0 +1,10 @@
+package algebra.laws.platform
+
+private[laws] object Platform {


since this is private, can we remove this and use:

https://github.com/typelevel/cats/blob/main/kernel-laws/jvm/src/main/scala/cats/platform/Platform.scala

I would have liked to, but that one is also private! Since they don't share the same package. Ok to make the cats-kernel one public?

johnynek · 2021-06-30T19:14:47Z

docs/src/main/mdoc/algebra.md

+|DistributiveLattice       |          ✓|          ✓|           |          |           ✓|          |                |
+|BoundedLattice            |          ✓|          ✓|          ✓|         ✓|            |          |                |
+|BoundedDistributiveLattice|          ✓|          ✓|          ✓|         ✓|           ✓|          |                |
+|Heyting                   |          ✓|          ✓|          ✓|         ✓|           ✓|         ✓|                |


I think Logic is missing from this table, btw...

If you tell me which boxes to check I'll add it in real quick :) I'm ok with the first 5 columns or so, but a bit confused about the last two.

armanbilge · 2021-06-30T20:23:42Z

@johnynek Thanks for the review, much appreciated. I've restored Priority to its rightful place.

@larsrh also expressed his overall approval, hopefully he can confirm here? :)

larsrh · 2021-07-03T06:52:09Z

I remain unconvinced that this one is better than #3877, but will not stand in the way of getting this done.

armanbilge · 2021-07-13T19:10:06Z

Just a polite bump :) does anyone else want to chime in? @denisrosset or @mpilquist?

Algebra is a part of cats as of typelevel/cats#3918

armanbilge added 11 commits May 1, 2021 04:17

Initial dump of algebra into cats

2c51138

Disable MiMa

71f397b

@nowarn deprecations

080a539

Migrate docs

700c0b7

Reference cats-algebra in README/docs

c51b726

Re-enable MiMa for algebra (but not laws)

d609b7d

Merge branch 'main' into migrate-algebra-bincompat

d54ae94

Brings in Scala 3 final

Merge in typelevel/algebra#246

a652b61

Implement typelevel/algebra#108

591b733

Add Signed and TruncatedDivision typeclasses

1ec9e21

Partially merges typelevel/algebra#247

Make Field inherit from DivisionRing

66a6231

Addresses typelevel/algebra#246 (comment)

armanbilge mentioned this pull request Jun 5, 2021

Move typelevel/algebra into cats repo #3877

Closed

armanbilge added 4 commits June 19, 2021 15:25

Start migrating tests

2943b54

Add tests for Signed/TruncatedDivision

5c4d170

Finishes merging typelevel/algebra#248

Prefer deprecation to source incompatibility

716ec2b

Remove redundant tests

ec0ed75

Fix reference to algebra in README

610d1e1

johnynek reviewed Jun 30, 2021

View reviewed changes

Un-deprecate Priority

84c780a

johnynek approved these changes Jul 1, 2021

View reviewed changes

armanbilge mentioned this pull request Aug 25, 2021

Call for maintainers typelevel/general#111

Closed

19 tasks

johnynek merged commit 41b99ed into typelevel:main Aug 25, 2021

joroKr21 mentioned this pull request Aug 26, 2021

Optimise boilerplate generators, use instance constructors #3871

Merged

gemelen mentioned this pull request Sep 10, 2021

1.0 typelevel/cats-collections#277

Open

armanbilge mentioned this pull request Sep 18, 2021

Move typelevel/algebra into cats repo. #2041

Closed

valencik added a commit to typelevel/typelevel.github.com that referenced this pull request Jan 26, 2023

Remove algebra

d7376cd

Algebra is a part of cats as of typelevel/cats#3918

valencik mentioned this pull request Jan 26, 2023

Remove algebra typelevel/typelevel.github.com#419

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Binary-compatibly move typelevel/algebra into cats repo #3918

Binary-compatibly move typelevel/algebra into cats repo #3918

armanbilge commented Jun 4, 2021 •

edited

Loading

armanbilge commented Jun 5, 2021

armanbilge commented Jun 19, 2021 •

edited

Loading

johnynek commented Jun 19, 2021

armanbilge commented Jun 19, 2021 •

edited

Loading

johnynek left a comment

johnynek Jun 30, 2021

johnynek Jun 30, 2021

armanbilge Jun 30, 2021

johnynek Jun 30, 2021

johnynek Jun 30, 2021

armanbilge Jun 30, 2021 •

edited

Loading

johnynek Jun 30, 2021

armanbilge Jun 30, 2021

armanbilge commented Jun 30, 2021

larsrh commented Jul 3, 2021

armanbilge commented Jul 13, 2021

		@@ -0,0 +1,10 @@
		package algebra.laws.platform

		private[laws] object Platform {

Binary-compatibly move typelevel/algebra into cats repo #3918

Binary-compatibly move typelevel/algebra into cats repo #3918

Conversation

armanbilge commented Jun 4, 2021 • edited Loading

armanbilge commented Jun 5, 2021

armanbilge commented Jun 19, 2021 • edited Loading

johnynek commented Jun 19, 2021

armanbilge commented Jun 19, 2021 • edited Loading

johnynek left a comment

Choose a reason for hiding this comment

johnynek Jun 30, 2021

Choose a reason for hiding this comment

johnynek Jun 30, 2021

Choose a reason for hiding this comment

armanbilge Jun 30, 2021

Choose a reason for hiding this comment

johnynek Jun 30, 2021

Choose a reason for hiding this comment

johnynek Jun 30, 2021

Choose a reason for hiding this comment

armanbilge Jun 30, 2021 • edited Loading

Choose a reason for hiding this comment

johnynek Jun 30, 2021

Choose a reason for hiding this comment

armanbilge Jun 30, 2021

Choose a reason for hiding this comment

armanbilge commented Jun 30, 2021

larsrh commented Jul 3, 2021

armanbilge commented Jul 13, 2021

armanbilge commented Jun 4, 2021 •

edited

Loading

armanbilge commented Jun 19, 2021 •

edited

Loading

armanbilge commented Jun 19, 2021 •

edited

Loading

armanbilge Jun 30, 2021 •

edited

Loading