You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@datasketches.apache.org by "tisonkun (via GitHub)" <gi...@apache.org> on 2026/07/28 11:09:56 UTC
[I] Run serde_test against datasketches-go snapshots (datasketches-rust)
tisonkun opened a new issue, #170:
URL: https://github.com/apache/datasketches-rust/issues/170
IIRC datasketches-go provides a generator to generate snapshots.
We may first add that generator to https://github.com/apache/datasketches-tck and then pull them in datasketches-rust to add test coverage for serde_test against them.
cc @proost @freakyzoidberg would you share some context how datasketches-go generates snapshots so that this issue can be even a good-first-issue for anyone equipped with Codex/Claude?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Run serde_test against datasketches-go snapshots (datasketches-rust)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on issue #170:
URL: https://github.com/apache/datasketches-rust/issues/170#issuecomment-5106390766
> In each sketches, there is a serialization test file. And that file, generating snapshot code is included. For example, [theta sketch](https://github.com/apache/datasketches-go/blob/main/theta/sketch_serialization_test.go#L36).
I get it now. Thank you! I'll try to include Go snapshots in datasketches-tck and may ask for your help in review.
> do those issues include C++ or Java code too? or rust only?
I remember something like:
* https://github.com/apache/datasketches-java/issues/701
* https://github.com/apache/datasketches-cpp/pull/505
* https://github.com/apache/datasketches-rust/issues/115
* https://github.com/apache/datasketches-rust/issues/133
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Run serde_test against datasketches-go snapshots (datasketches-rust)
Posted by "proost (via GitHub)" <gi...@apache.org>.
proost commented on issue #170:
URL: https://github.com/apache/datasketches-rust/issues/170#issuecomment-5106650764
@tisonkun
I am aware of those compatibility issues. When they were discovered, I and other members of the datasketches-go community also worked on fixing the relevant issues. As far as I know, there are currently no outstanding compatibility issues involving datasketches-go.
I will continue to follow this issue as well. If you find any compatibility issue related to datasketches-go, please leave a comment here, and I will help investigate and resolve it with you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Run serde_test against datasketches-go snapshots (datasketches-rust)
Posted by "proost (via GitHub)" <gi...@apache.org>.
proost commented on issue #170:
URL: https://github.com/apache/datasketches-rust/issues/170#issuecomment-5106290004
> Agree. Could you share the code location or a link to the snapshot generation code?
In each sketches, there is a serialization test file. And that file, generating snapshot code is included. For example, [theta sketch](https://github.com/apache/datasketches-go/blob/main/theta/sketch_serialization_test.go#L36).
> What do you mean in this sentence? I don't get your point and it seems unrelated to other parts.
Sorry, I phrased that poorly. I was only qualifying my earlier statement that all compatibility work should be completed before the first release containing the serialization/deserialization support. Since datasketches-rust is still pre-1.0, even if serialization code is released and we later discover through additional cross-language tests that the format is incorrect, we still have more room to fix it with a breaking change. The backward compatibility commitment is not as strong as it would be after 1.0.0. (Of course, more stricter is better).
> And we did find several serde compatibility issues and got them fixed.
do those issues include C++ or Java code too? or rust only?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Run serde_test against datasketches-go snapshots (datasketches-rust)
Posted by "proost (via GitHub)" <gi...@apache.org>.
proost commented on issue #170:
URL: https://github.com/apache/datasketches-rust/issues/170#issuecomment-5106833059
If you ok, i will add a new make command for generating snapshots only. how do you think? does make it easier generating snapshots in the tck?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Run serde_test against datasketches-go snapshots (datasketches-rust)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun closed issue #170: Run serde_test against datasketches-go snapshots
URL: https://github.com/apache/datasketches-rust/issues/170
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Run serde_test against datasketches-go snapshots (datasketches-rust)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on issue #170:
URL: https://github.com/apache/datasketches-rust/issues/170#issuecomment-5105981586
@proost Thanks for your information! Comments inline.
> I think implementing the snapshot generation code itself is relatively straightforward. The more difficult part for a first-time contributor is figuring out which cases need to be covered and why. That usually requires understanding the sketch's internal, serialization format, and the cases that may affect cross-language compatibility.
Agree. Could you share the code location or a link to the snapshot generation code? I'm going to make a go generator like [the cpp generator](https://github.com/apache/datasketches-tck/blob/main/serialization/cpp/generator.py) to check in Go snapshots in the datasketches-tck repo so that they can be used in any other datasketches impls.
> However, datasketches-rust has not released v1.0.0 yet, so I think it is reasonable to be somewhat less strict during the pre-1.0 development phase. User and community understand that point :)
What do you mean in this sentence? I don't get your point and it seems unrelated to other parts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Run serde_test against datasketches-go snapshots (datasketches-rust)
Posted by "proost (via GitHub)" <gi...@apache.org>.
proost commented on issue #170:
URL: https://github.com/apache/datasketches-rust/issues/170#issuecomment-5105929688
@tisonkun
In datasketches-go, when we add serialization/deserialization support, we generally include cross-language compatibility test code and snapshot generation code. We may also include generated sketch binaries, although committing the binaries themselves is optional as long as they can be reproduced from the generator.
The compatibility test code is the important part. It should include the snapshot generation logic so that we can understand how each snapshot was produced and regenerate it when necessary.
I think implementing the snapshot generation code itself is relatively straightforward. The more difficult part for a first-time contributor is figuring out which cases need to be covered and why. That usually requires understanding the sketch's internal, serialization format, and the cases that may affect cross-language compatibility.
That said, I do not think all of this work necessarily needs to be included in a single PR. Serialization/deserialization support, snapshot generation, generated binaries, and cross-language compatibility tests can be split into separate PRs or issues, as long as maintainers track the related work.
However, all of them should be completed before the first release that includes the new serialization/deserialization support. Cross-language compatibility is one of the core properties of DataSketches. If serialization support is released before the compatibility tests are in place, we may release an incompatible binary format and discover the problem only after users have already produced release.
So I think this can be a good first issue if maintainers provide guidance about the required test cases and make sure the remaining compatibility work is tracked as a release requirement.
------
However, datasketches-rust has not released v1.0.0 yet, so I think it is reasonable to be somewhat less strict during the pre-1.0 development phase. User and community understand that point :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Run serde_test against datasketches-go snapshots (datasketches-rust)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on issue #170:
URL: https://github.com/apache/datasketches-rust/issues/170#issuecomment-5106009449
@proost In datasketches-rust, we have [serde_tests](https://github.com/apache/datasketches-rust/blob/d6cc6d8f6f45456ae975fe5af4883c595e0021d2/datasketches/tests/serde_tests.rs) to test against C++ and Java snapshots. And we did find several serde compatibility issues and got them fixed.
Now we are going to include Go snapshot to help all the datasketches impls provide the same serde compatibility promise. This is what this issue is for.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Run serde_test against datasketches-go snapshots (datasketches-rust)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on issue #170:
URL: https://github.com/apache/datasketches-rust/issues/170#issuecomment-5111645738
> If you ok, i will add a new make command for generating snapshots only. how do you think? does make it easier generating snapshots in the tck?
Yes that can be helpful, while let the Python script do what the command in the Makefile does is OK as well.
You can add a new make command. It helps.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Run serde_test against datasketches-go snapshots (datasketches-rust)
Posted by "proost (via GitHub)" <gi...@apache.org>.
proost commented on issue #170:
URL: https://github.com/apache/datasketches-rust/issues/170#issuecomment-5118985296
I added new make command. If run "generate-go-snapshots", then all the snapshots are created into `/serialization_test_data/go_generated_files`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org