Skip to content

Some work on formalizing Suffix (re: #517) #551

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

Merged
merged 12 commits into from
Jan 12, 2019
Merged

Conversation

iitalics
Copy link
Contributor

Hello, new Agda contributor, so please let me know if I have made any major style violations or anything like that.

Here is an implementation of a heterogeneous Suffix property, to go along with Prefix (#522). Many of the Prefix theorems are mirrored here, except for those that don't apply easily to suffixes (e.g. zipWith, inits, ...).

Considerations before merging:

  • The data constructor names here/there may not be appropriate, maybe something like skip/stop is more accurate since here does not refer to a single element, like in the case of Any.
  • I've defined a number of Pointwise lemmas that probably deserve to be moved into the Pointwise module.

@gallais gallais added this to the v0.18 milestone Nov 30, 2018

module _ {a b r} {A : Set a} {B : Set b} {R : REL A B r} where

-- TODO: move this lemma (and others) into Pointwise?
Copy link
Member

Choose a reason for hiding this comment

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

Yes! It would be nice if these were expressed in terms of reverseAcc
which I am introducing in #529.

So pw-reverse would be reverse⁺ and foldl would be reverseAcc⁺.

foldl rs′ [] = rs′
foldl rs′ (r ∷ rs) = foldl (r ∷ rs′) rs

fromPrefix⁺ : ∀ {as bs} → Prefix R as bs → Suffix R (reverse as) (reverse bs)
Copy link
Member

Choose a reason for hiding this comment

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

This seems like fromPrefix to me (no need for the as the conclusion
does not mention a function called fromPrefix).

It should probably go in Suffix.Heterogeneous proper rather than X.Properties.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see. The problem is that toPrefix⁻ is eerily similar. Hmm.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I agree with @gallais that these names don't obey the conventions used elsewhere. I think better names for these lemmas might be fromPrefix, fromPrefix-rev, toPrefix-rev and toPrefix respectively...

{R : REL A B r} {S : REL B A s} {E : REL A B e} where

private
pw-antisym : Antisym R S E → Antisym (Pointwise R) (Pointwise S) (Pointwise E)
Copy link
Member

Choose a reason for hiding this comment

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

This should also go in Pointwise. May be worth creating Pointwise.Properties now.

Copy link
Member

Choose a reason for hiding this comment

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

Same for pw-map⁺, etc.

@iitalics
Copy link
Contributor Author

OK I have a local branch that moves the Pointwise lemmas into Data.List.Relation.Pointwise, but it's predicated on reverseAcc from #529.

@gallais gallais added the status: blocked-by-issue Progress on this issue or PR is blocked by another issue. label Dec 7, 2018
@MatthewDaggitt
Copy link
Contributor

@iitalics I've just merged #529 so feel free to update this when you get the chance.

@MatthewDaggitt MatthewDaggitt added status:not-ready-yet and removed status: blocked-by-issue Progress on this issue or PR is blocked by another issue. labels Dec 24, 2018
@iitalics
Copy link
Contributor Author

OK, rebased and refactored.


module _ {a b r} {A : Set a} {B : Set b} {R : REL A B r} {as} where

length-mono-Suffix-≤ : ∀ {bs} → Suffix R as bs → length as ≤ length bs
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could just be called length-mono-≤ or even length-mono

Copy link
Member

Choose a reason for hiding this comment

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

I'm to blame for this one:

https://agda.github.io/agda-stdlib/Data.List.Relation.Prefix.Heterogeneous.Properties.html#2512

(and I have a similar one for the upcoming Sublist in #562)

Should we change all of these to length-mono?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes please, I think we probably should. I think the convention is not to reference the data type within the properties file.

@MatthewDaggitt
Copy link
Contributor

Thanks, looks great. I've a last couple of comments and then I'll merge it in.

@MatthewDaggitt MatthewDaggitt merged commit 237a0bd into agda:master Jan 12, 2019
@MatthewDaggitt
Copy link
Contributor

MatthewDaggitt commented Jan 12, 2019

I've merged in. I'll perform the tweaks myself. Thanks for the PR @iitalics.

@iitalics iitalics deleted the suffix branch January 23, 2019 15:26
@MatthewDaggitt MatthewDaggitt modified the milestones: v0.18, v1.0 Feb 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants