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

Deprecate upickle-pprint #209

Closed
lihaoyi opened this issue May 12, 2017 · 11 comments
Closed

Deprecate upickle-pprint #209

lihaoyi opened this issue May 12, 2017 · 11 comments

Comments

@lihaoyi
Copy link
Member

lihaoyi commented May 12, 2017

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 com-lihaoyi/Ammonite#575, #196, #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:

  • Streaming reading/writing of JSON from e.g. java.io.{Input,Output}Streams
  • "deep" xpath-style lookup, a better syntax for creating JSON fragments
  • Better error messages for parsing and validation

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

@MasseGuillaume
Copy link
Contributor

@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.

@lihaoyi
Copy link
Member Author

lihaoyi commented May 12, 2017

@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.

@MasseGuillaume
Copy link
Contributor

MasseGuillaume commented May 12, 2017

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

@lihaoyi
Copy link
Member Author

lihaoyi commented May 12, 2017

@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

@lihaoyi
Copy link
Member Author

lihaoyi commented May 17, 2017

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

@bwbecker
Copy link
Contributor

@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.

@zoosky
Copy link

zoosky commented May 25, 2017

Have a look at kyleu/solitaire.gg@81be62e

@lihaoyi
Copy link
Member Author

lihaoyi commented Dec 19, 2017

I decided to un-deprecate it, and released 0.5.1

@lihaoyi lihaoyi closed this as completed Dec 19, 2017
@LogicalTime
Copy link

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. :-)

@SethTisue
Copy link

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

@LogicalTime
Copy link

@SethTisue thank you kindly

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

No branches or pull requests

6 participants