random icon indicating copy to clipboard operation
random copied to clipboard

Add `instance Random DiffTime`

Open domenkozar opened this issue 1 year ago • 3 comments

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.

domenkozar avatar Jan 12 '24 10:01 domenkozar

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.)

lehins avatar Jan 12 '24 14:01 lehins

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?

Bodigrim avatar Jan 12 '24 19:01 Bodigrim

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.

lehins avatar Jan 12 '24 19:01 lehins