Skip to content
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

Add instance Random DiffTime #156

Open
domenkozar opened this issue Jan 12, 2024 · 3 comments
Open

Add instance Random DiffTime #156

domenkozar opened this issue Jan 12, 2024 · 3 comments

Comments

@domenkozar
Copy link

instance Random DiffTime where
  random :: (RandomGen g) => g -> (DiffTime, g)
  random g =
    let (i, g') = random g
     in (picosecondsToDiffTime $ fromIntegral (i :: Integer), g')

  randomR :: RandomGen g => (DiffTime, DiffTime) -> g -> (DiffTime, g)
  randomR (a, b) g =
    let (i, g') = randomR (diffTimeToPicoseconds a, diffTimeToPicoseconds b) g
     in (picosecondsToDiffTime i, g')

Note that the instance can't go into time because of random -> splitmix -> time dependency chain.

@lehins
Copy link
Contributor

lehins commented Jan 12, 2024

This is not a good instance because it depends on an infinite Integer. I can only see us adding an instance for UniformRange type class. This would be sufficient IMHO, because uniformR and uniformRM could then be used to generate DiffTime in the desired range.

I personally would not mind adding time as dependency in order to provide instances for other types from that core library (UTCTime, NominalDiffTime, Day, etc.)

@Bodigrim
Copy link
Contributor

As someone working on boot libraries, I'm reluctant for random to acquire another dependency. This would make running QuickCheck tests for boot libraries with GHC HEAD even more convoluted and constrained. I'm already unhappy with the dependency on bytestring, but time entailing Win32, filepath, exceptions, etc. will be a massive headache.

Shall we split the package into random and random-instances? Or random-core / random-types (using only types from base) and random proper?

@lehins
Copy link
Contributor

lehins commented Jan 12, 2024

Yeah, that's true. random is essentially a must for testing any package due to quickcheck.

I think splitting out random into two packages random and random-core would be a good idea. This would solve the dependency on bytestring as well since we can now rely on byte-array. It will require a bit of planning, but I think it is a good direction.

This way we can have QuickCheck depend on random-core, instead of random. Latter would have less restriction on what it depends on, thus it would be able to provide instances for more core packages.

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

No branches or pull requests

3 participants