Use spurious Fractional[MiniInt] to test Invariant[Fractional]
#4216
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently we use Float when doing laws testing for Invariant[Fractional]. This in turn requires us to generate an Eq[Fractional[Float]], which can only be done using DeprecatedEqInstances.catsLawsEqForFn1. In order to stop using this deprecated instances, in this change we now use MiniInt to do our laws testing for Invariant[Fractional]. Note that this approach is spurious since MiniInt is not a fractional number type. But we can use it to test the lawfulness of the Invariant[Fractional] instance, and there is no "MiniFloat" type that is both small enough to have an ExhaustiveCheck instance and fractional. See typelevel#4033 for a broader discussion of this.
tmccarthy
commented
May 31, 2022
Comment on lines
333
to
336
| checkAll("Invariant[Numeric]", InvariantTests[Numeric].invariant[MiniInt, Boolean, Boolean]) | ||
| checkAll("Invariant[Integral]", InvariantTests[Integral].invariant[MiniInt, Boolean, Boolean]) | ||
| checkAll("Invariant[Fractional]", InvariantTests[Fractional].invariant[MiniInt, Boolean, Boolean]) | ||
|
|
There was a problem hiding this comment.
Moved these down here so that they are grouped with the rest of the Invariant laws tests.
armanbilge
reviewed
Jun 4, 2022
| implicit private val arbFractionalFloat: Arbitrary[Fractional[Float]] = Arbitrary( | ||
| Gen.const(implicitly[Fractional[Float]]) | ||
| ) | ||
| implicit private val arbFractionalFloat: Arbitrary[Fractional[MiniInt]] = Arbitrary(Gen.const(fractionalForMiniInt)) |
There was a problem hiding this comment.
Since this is a spurious instance, probably better to pass it explicitly than have it floating around in implicit scope. Also would be good to add a comment to that effect.
Suggested change
| implicit private val arbFractionalFloat: Arbitrary[Fractional[MiniInt]] = Arbitrary(Gen.const(fractionalForMiniInt)) | |
| private val arbFractionalMiniInt: Arbitrary[Fractional[MiniInt]] = Arbitrary(Gen.const(fractionalForMiniInt)) |
There was a problem hiding this comment.
Alternatively, maybe define it only at its use site? e.g.
{
implicit private val arbFractionalMiniInt = ???
checkAll("Invariant[Fractional]", InvariantTests[Fractional].invariant[MiniInt, Boolean, Boolean])
}
armanbilge
approved these changes
Jun 5, 2022
armanbilge
changed the title
Fractional[MiniInt] to test Invariant[Fractional]
|
Thanks @armanbilge for working with me on this one! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently we use
Floatwhen doing laws testing forInvariant[Fractional]inAlgebraInvariantSuite. This in turn requires us to define anEq[Fractional[Float]], which can only be done usingDeprecatedEqInstances.catsLawsEqForFn1(here). In order to stop using this deprecated instances, in this change we now useMiniIntto do our laws testing forInvariant[Fractional]. BecauseMiniInthas anExhaustiveCheckinstance, we no longer have to use the deprecatedEq[A => B]instance.Note that this approach is spurious since
MiniIntis not a fractional number type. But we can use it to test the lawfulness of theInvariant[Fractional]instance, and there is no "MiniFloat" type that is both small enough to have anExhaustiveCheckinstance and fractional. See #4033 for a broader discussion of this.