-
Notifications
You must be signed in to change notification settings - Fork 73
Update cabal + add wrapper over MWC #225
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
Conversation
Shimuuar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There're few comments but otherwise it's good
No. Tests are not needed. It's just a wrapper and generator is tested in mwc-random
Statistics/Distribution/Poisson.hs
Outdated
| import qualified Statistics.Distribution as D | ||
| import qualified Statistics.Distribution.Poisson.Internal as I | ||
| import Statistics.Internal | ||
| import Control.Monad (liftM) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this import needed?
| instance D.ContGen PoissonDistribution where | ||
| genContVar (PD lambda) gen = fromIntegral <$> MWC.poisson lambda gen | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please drop ContGen instance. It is discrete distribution after all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @Shimuuar , i am not able to drop ContGen:
class (DiscreteDistr d, ContGen d) => DiscreteGen d where
genDiscreteVar :: (StatefulGen g m) => d -> g -> m Int
statistics/Statistics/Distribution.hs
Line 168 in b628309
| class (DiscreteDistr d, ContGen d) => DiscreteGen d where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed! Sorry I forgot about it
|
@Shimuuar , i have removed the unnecessary import. |
|
Thank you. 0.16.5.0 is on hackage P.S. Generally it's better to make PR from branches other than master. No need to reset master branch afterwards |
Hi, do we need tests for this?