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 inference rule for Greater[A] ==> Not[Less[A]] #452

Merged
merged 3 commits into from Mar 13, 2018

Conversation

zainab-ali
Copy link
Contributor

If a value is greater than x, then it also must not be less than x.

What do you think of introducing it as an inference rule?

@zainab-ali zainab-ali changed the title Add inference rule for Greater[A] ==> Not[Less[A]] (WIP) Add inference rule for Greater[A] ==> Not[Less[A]] Mar 10, 2018
@zainab-ali
Copy link
Contributor Author

The binary compatability check is failing in 2.10 and 2.11 due to the addition of the new method

@fthomas
Copy link
Owner

fthomas commented Mar 10, 2018

This LGTM, thanks @zainab-ali! What do you think about using GreaterEqual[A] (which is an alias for Not[Less[A]]) for the type of that rule? I think Greater[A] ==> GreaterEqual[A] is easier to understand.

If we add this, it would also make sense to add Less[A] ==> LessEqual[A]. We can add this in a later PR, no need to do this here.

With regards to the MiMa checks: the next version will be 0.9.0 so it is okay to break binary compatability. Just add MiMa exclusions here to ignore those errors.

@codecov
Copy link

codecov bot commented Mar 12, 2018

Codecov Report

Merging #452 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #452      +/-   ##
==========================================
+ Coverage   98.03%   98.03%   +<.01%     
==========================================
  Files          57       57              
  Lines         660      662       +2     
  Branches       12       12              
==========================================
+ Hits          647      649       +2     
  Misses         13       13
Impacted Files Coverage Δ
...ed/src/main/scala/eu/timepit/refined/numeric.scala 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7080ca0...36acee1. Read the comment docs.

@zainab-ali
Copy link
Contributor Author

zainab-ali commented Mar 12, 2018

Right, GreaterEqual is easier to reason about. I've thrown LessEqual too, since it was straightforward.

Integrating mima and scalafmt into the build like this is a great idea :) I'll start doing that for my own projects!

EDIT: There are still some mima exclusions I need to add

Ok, exclusions added

Copy link
Owner

@fthomas fthomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks again @zainab-ali!

@fthomas fthomas merged commit 297d3e5 into fthomas:master Mar 13, 2018
kwark added a commit to kwark/play-refined that referenced this pull request Apr 29, 2018
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

Successfully merging this pull request may close these issues.

None yet

2 participants