support custom serialization for datasets, remote evals, and scorers#136
Conversation
Andrew Kent (realark)
force-pushed
the
ark/dataset-generics
branch
2 times, most recently
from
881a8c7 to
6abf086
Compare
Andrew Kent (realark)
changed the title
Andrew Kent (realark)
force-pushed
the
ark/dataset-generics
branch
6 times, most recently
from
9c17005 to
a3d2460
Compare
in several places of the codebase we lost type info due to erasure which made it impossible to ser/de complex objects which fetching json from the braintrust api. Our examples used simple types jackson could handle out of the box, which is why we didn't hit this before. This change introduces type info into the public apis for fetching objects from braintrust. It also includes options for custom serializers (and by default uses the global braintrust mapper) affected: - remote datasets - remote scorers - devserver
Andrew Kent (realark)
force-pushed
the
ark/dataset-generics
branch
from
a3d2460 to
23cc5ac
Compare
David Elner (delner)
left a comment
David Elner (delner)
left a comment
There was a problem hiding this comment.
Just a question about deprecations. If we don't have a plan for how deprecations are handled, maybe we need one? Not a blocker, but food for thought as a follow up.
| * {@link #fetchFromBraintrust(BraintrustOpenApiClient, String, String, String, Class, | ||
| * Class)} instead. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
Just curious, what's the reason behind this deprecation? What's its blast radius? When does it get removed?
From what I can see you now require some kind of templated input/output? Makes sense for "custom serialization" just wondering how this connects to the deprecation.
There was a problem hiding this comment.
The deprecation is to encourage people to explicitly type their inputs/outputs using the newer methods. This deprecated method can produce serialization bugs if the input/output types are not simple (e.g. String will work but not MyFoobarThing).
In terms of when that gets removed, there currently isn't a process for that. I'd be curious to know how other SDKs handle this. My thinking is when we get around to cleaning up deprecated methods we'll mark them for explicit removal in an upcoming release and wait a reasonable amount of time for for users to change their code.
There was a problem hiding this comment.
we should only remove in a major. If the SDK is pre 1.0, I think we wait at least a quarter before removing a smaller feature. For something bigger like this which has been around for a while, I'd be comfortable to waiting until the SDK is 1.0 before removing.
3 participants
in several places of the codebase we lost type info due to erasure which made it impossible to ser/de complex objects when fetching json from the braintrust api. Our examples used simple types jackson could handle out of the box, which is why we didn't hit this before. This change introduces type info into the public apis for fetching objects from braintrust. It also includes options for custom serializers (and by default uses the global braintrust mapper)
fixes #111
affected:
Working on this inspired a few follow-up issues: