Skip to content
This repository was archived by the owner on Feb 8, 2025. It is now read-only.

Make Make ZipfDistribution generic over T and impl Distribution<T> #6

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vtta
Copy link

@vtta vtta commented Dec 10, 2022

Previously ZipfDistribution only generate values of usize. This patch introduce a generic type parameter T to allow ZipfDistribution generating any primitive type value.

This patch makes this crate more helpful.

@codecov-commenter
Copy link

Codecov Report

Base: 89.04% // Head: 56.94% // Decreases project coverage by -32.09% ⚠️

Coverage data is based on head (cd627cc) compared to base (f6dddc8).
Patch coverage: 60.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           master       #6       +/-   ##
===========================================
- Coverage   89.04%   56.94%   -32.10%     
===========================================
  Files           1        1               
  Lines          73       72        -1     
===========================================
- Hits           65       41       -24     
- Misses          8       31       +23     
Impacted Files Coverage Δ
src/lib.rs 56.94% <60.00%> (-32.10%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jonhoo
Copy link
Owner

jonhoo commented Dec 11, 2022

Thank you for taking the time to write up a PR!

I'd actually prefer to do this without bringing in num as a dependency. It's mostly unnecessary, and also still fairly limiting (e.g., it won't give us things like generating tuples). Instead, I think the way rand expects this to be done is by distributions implementing the Distribution trait once for each type it supports generating (see the impl list). And much of that can be done easily with macros (see what rand does). Would you want to give adding some impls like that a try?

Separately, I actually don't know if this algorithm is actually reasonable to use for generating non-integers. I forget what the original paper stipulated, but I think it was just intended for integer values. Worth double-checking. Also, using this to generate, say, char is going to be pretty weird :p

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants