[ refactor ] Revise definitions, consequences, and use, of Algebra.Definitions.(Almost)*Cancellative
#2573
Conversation
|
I think I can't face doing more with this PR at this stage: the new definitions are in-place, with one specimen instantiation for
without a lot more refactoring, which I suggest take place downstream? |
Thanks @JacquesCarette I suppose my concern was that if it turned out those downstream PRs were still stymied by 'bad' design decisions here, then ... I'm not sure what. Certainly it seemed a big job to try to refactor existing proofs to try to conform to the new APIs... |
|
To allay your fears, might be worth taking |
So to allay the fears about whether it's too much work to make the change, I may as well (accept having to) do the additional work to make the change? Sigh... |
Well, it might turn out to be easy! Or you might be right, and be able to estimate just how much work it is, and let us know. |
| @@ -3,7 +3,7 @@ | |||
| module README where | |||
|
|
|||
| ------------------------------------------------------------------------ | |||
| -- The Agda standard library, version 2.3-dev | |||
| -- The Agda standard library, version 3.0-dev | |||
There was a problem hiding this comment.
This change probably shouldn't be made in this PR?
| * In `Data.Nat.Properties`: | ||
| ```agda | ||
| *-almostCancelʳ-≡ : AlmostRightCancellative 0 _*_ | ||
| ``` |
There was a problem hiding this comment.
Seem to be missing a lot of stuff in the CHANGELOG?
|
|
||
| Cancellative : Op₂ A → Set _ | ||
| Cancellative _•_ = (LeftCancellative _•_) × (RightCancellative _•_) | ||
|
|
||
| _-AlmostLeftCancellative_ Provided_-LeftCancellative_ | ||
| Except_-LeftCancellative_ : ∀ {p} (P : Pred A p) → Op₂ A → Set _ |
There was a problem hiding this comment.
These lines are pretty unreadable to me. Can we separate them out into separate definitions even if they do have the same type?
Also why do these names have - in them?
| @@ -44,6 +43,7 @@ import Relation.Binary.Properties.Poset as PosetProperties | |||
| import Relation.Binary.Reasoning.Setoid as ≈-Reasoning | |||
| open import Relation.Binary.Reasoning.Syntax | |||
|
|
|||
| open import Algebra.Consequences.Propositional {A = ℚᵘ} | |||
There was a problem hiding this comment.
This seems like an unneeded import?
| @@ -64,6 +63,7 @@ open import Relation.Nullary.Decidable.Core as Dec | |||
| using (yes; no; recompute; map′; _×-dec_) | |||
| open import Relation.Nullary.Negation.Core using (¬_; contradiction) | |||
|
|
|||
| open import Algebra.Consequences.Propositional {A = ℚ} | |||
There was a problem hiding this comment.
Why do we now need to import it with the type specifically applied? Is this a breaking change?
Fixes #1436 , ...
... at least as far as 'AlmostCancellative' properties of operations wrt equality relation; the various order-theoretic generalisations are not covered
... so the issue probably ought to be kept open even if this gets merged? cf. #2580
NB.:
breaking?namingissues... etc.)Algebra.Consequences.Baserefactoring [ refactor ] Add equality as a parameter toAlgebra.Consequences.Base#2572 should probably be merged first; fold in theinstance-based analysis forDecidable(and henceRecomputable) predicates intoExcept_-*Cancellative_?Algebra.Definitionsonly imported at the equality relation, but might better be left parameterised, so that the variousCancellativeproperties can be made generic...IntegerandRational, theCancellativeproperties wrt multiplication don't easily cash out via the 'generic' design strategy of going viaAlmostCancellative, so I haven't (yet) include such statements, nor refactored to make these the primary notions.