search

BurntSushi / rust-stats

Basic statistical functions on streams for Rust.
The Unlicense
87 stars 18 forks source link

Optional serde support, extra trait impls, rustfmt and clippy #17

Closed Fuuzetsu closed 3 years ago

Fuuzetsu commented 3 years ago

See individual commits. The first few (rustfmt, clippy) are just convenience for subsequent editing. The serde commit is the main addition.

Fuuzetsu commented 3 years ago

Hm actually this doesn't compile with feature on, let me try to fix that...

BurntSushi commented 3 years ago

Before submitting big PRs like this---particularly ones that modify the public API---it's generally a good idea to file an issue first to see if it would be wanted.

This crate isn't really something I'm actively developing any more. Moreover, adding Serde support like this doesn't really seem like a good fit IMO for a variety of reasons. First is that these structures are mostly intended to just be kept in memory. Second is that once a serialized form is supported, breaking that support is a semver breaking change. That means that any changes to the internal data structure would likely require writing a custom Deserialize impl to support the older format.

Probably my advice to you is to not use this crate. It's really not doing much for you.

Fuuzetsu commented 3 years ago

Before submitting big PRs like this---particularly ones that modify the public API---it's generally a good idea to file an issue first to see if it would be wanted.

These are changes we are using anyway so it was very little cost to send them.

Probably my advice to you is to not use this crate. It's really not doing much for you.

Alright, I'll keep using the fork and/or eventually make own version or inline the bits we need or trying to updating rolling-stats crate instead.

Thanks for your time.