search

amnaredo / test

0 stars 0 forks source link

Deprecate upickle-pprint #214

Open amnaredo opened 3 years ago

amnaredo commented 3 years ago

These two libraries have had a good run over the last 3 years, but many of their original motivations for existing have since disappeared, and other parts of their design haven't work out over time.

Automatic, deep compile-time derivation

The current implementation, in the derive/ subproject, is probably a dead end in terms of engineering: it is a rats nest of complexity that works ok for what it does now, but nobody understands how it works, and it is almost impossible to fix the existing bugs without causing regressions (i.e. the code is a total disaster).

This results in bugs like https://github.com/lihaoyi/Ammonite/issues/575, https://github.com/lihaoyi/upickle-pprint/issues/196, https://github.com/lihaoyi/upickle-pprint/issues/182, and may others that are beyond my ability to fix.

For JSON serialization, a partial workaround is to declare implicit read-writers in every case class's companion objects via implicit val rw = upickle.default.macroRW[MyClass]. This helps avoid the nasty/buggy deep derivation logic entirely, and is something I have adopted in all my own projects. We could make this easier with a macro annotation, but fundamentally if we're giving up on automatic deep compile-time derivation, we could just use something like https://github.com/playframework/play-json instead.

If we want to try a more principled approach to compile-time derivation, we should probably use Shapeless instead of doing it ourselves. In that case, we could also just use https://github.com/circe/circe which seems to work out of the box., or https://github.com/julienrf/play-json-derived-codecs

For pretty-printing, my main use case is for Ammonite on the JVM. In that case, using runtime/Java reflection is probably the right thing to do.

Scala-js Compatibility

uPickle was the first, and at the time only, cross-JS/JVM serialization library. Since then there have been countless others, including new ones like https://github.com/circe/circe and existing ones like https://github.com/playframework/play-json which have picked up cross-platform support.

Those other libraries are better maintained and more featureful, so since uPickle is no longer the only one with Scala.js support, we should probably focus our efforts/maintenance/contributions on one of the others.

I don't really use PPrint much on Scala.js, so a Java-reflection-based PPrint that doesn't support Scala.js isn't an issue for me.

Simple API

Part of uPickle's benefits is the simple API for serializing things: 

import upickle.default._

write(1)                          ==> "1"

write(Seq(1, 2, 3))               ==> "[1,2,3]"

read[Seq[Int]]("[1, 2, 3]")       ==> List(1, 2, 3)

write((1, "omg", true))           ==> """[1,"omg",true]"""

And for looking things up:

json(8)("real").value == -9876.54321

Play-Json more or less already does the same thing:

@ Json.stringify(Json.toJson(1))
res4: String = "1"
@ Json.stringify(Json.toJson(Seq(1, 2, 3)))
res5: String = "[1,2,3]"
@ Json.fromJson[Seq[Int]](Json.parse("[1,2,3]"))
res6: play.api.libs.json.JsResult[Seq[Int]] = JsSuccess(List(1, 2, 3), )

While the API is more verbose, it should be trivial to add a helper function to simplify it. Other things, like tuples not being supported:

@ Json.stringify(Json.toJson((1, "omg", true)))
cmd7.sc:1: No Json serializer found for type (Int, String, Boolean). Try to implement an implicit Writes or Format for this type.
val res7 = Json.stringify(Json.toJson((1, "omg", true)))
                                     ^
Compilation Failed

Should also be easy to add.

Similarly, play-json has a somewhat inconsistent .apply/\ based API, but that can also be fixed relatively easily to provide the same lookup-syntax that uPickle currently has:

Overall, play-json is probably similar enough to uPickle at this point, with the new Scala.js support, that it might be easier to start from it as a base and port the simple API on top of it, rather than trying to wrangle uPickle's internals into a simpler state. I'm not familiar enough with Circe to say whether or not it's the same, but likely they solved many of the same problems, and it may also be easier to prettify circe rather than cleaning up uPickle's internals

Additional Features

It would be nice to provide things like:

These are all things that Circe/Play-Json provide to varying degrees, and it would be nice if we could leverage that rather than re-implementing all the same stuff in uPickle

Overall, uPickle and PPrint have had a good run over the last 3 years, but I think the design of these two libraries is probably a dead end. The best way forward is probably to port all the syntactic niceties over to play-json, whether upstream or as a fork, and implementing a Java-reflection-based pretty-printer for use in Ammonite.

Things that would make play-json work as a mostly-drop-in replacement for uPickle:

There may be a few others, but it's a relatively short list that would make play-json work for 99% of uPickle's use cases ID: 209 Original Author: lihaoyi

amnaredo commented 3 years ago

@lihaoyi well first off thank you for this amazing library. I use it in Scalakata and in Scastie for instrumentation rendering for both value (pprint) and type (tprint).

TPrint have no other alternative. If you want, I can extract it from this repository and maintain it. Original Author: MasseGuillaume

amnaredo commented 3 years ago

@MasseGuillaume good point about TPrint, I had totally forgotten about it!

I think that is a pretty standalone project, and doesn't interact with the "rest" of PPrint much if at all. extracting it into a separate repository would probably make the most sense. I will likely continue using it in Ammonite as well. Original Author: lihaoyi

amnaredo commented 3 years ago

@lihaoyi can you create a repo under you GitHub username with a blank READM?. I can create a PR to extract it. Original Author: MasseGuillaume

amnaredo commented 3 years ago

@MasseGuillaume I have given you contributor access to https://github.com/lihaoyi/TPrint.

Let me know if you want publishing credentials and I can give those to you too Original Author: lihaoyi

amnaredo commented 3 years ago

PPrint has been broken out into https://github.com/lihaoyi/pprint, with 0.5.1 released Original Author: lihaoyi

amnaredo commented 3 years ago

@lihaoyi Thanks for this library! I agree, it's had a great run for 3 years and I've used it often. Sorry to see it go. Original Author: bwbecker

amnaredo commented 3 years ago

Have a look at https://github.com/KyleU/solitaire.gg/commit/81be62e439be90e2efaaeb875c1ac7b6f8427d9c Original Author: zoosky

amnaredo commented 3 years ago

I decided to un-deprecate it, and released 0.5.1 Original Author: lihaoyi

amnaredo commented 3 years ago

Lol, you made a good argument for deprecating but no argument for undeprecating. How should I decide whether to finish migrating to Circe or not. :-) Original Author: LogicalTime

amnaredo commented 3 years ago

@LogicalTime see http://www.lihaoyi.com/post/uJsonfastflexibleandintuitiveJSONforScala.html Original Author: SethTisue

amnaredo commented 3 years ago

@SethTisue thank you kindly Original Author: LogicalTime