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

[stdlib] filter_length #17027

Merged
merged 1 commit into from
Dec 29, 2022
Merged

[stdlib] filter_length #17027

merged 1 commit into from
Dec 29, 2022

Conversation

haansn08
Copy link
Contributor

Adds filter_length: length (filter l) <= length l.

@haansn08 haansn08 requested a review from a team as a code owner December 27, 2022 02:12
@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Dec 27, 2022
Copy link
Contributor

@andres-erbsen andres-erbsen left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution. I left some suggestions inline.

theories/Lists/List.v Outdated Show resolved Hide resolved
@andres-erbsen
Copy link
Contributor

@coqbot run ci

@andres-erbsen andres-erbsen self-assigned this Dec 27, 2022
@JasonGross
Copy link
Member

Do we also want something like length (filter f l) + length (filter (fun x => negb (f x)) l) = length l?

@andres-erbsen
Copy link
Contributor

I agree that accepting a mere inequality as "the" lemma for length of filter seems dissatisfying in some way, but I don't see where I'd want a lemma where the implied length is expressed in terms of filter itself or using essentially a reimplementation of it such as sum (map Nat.of_bool . ). Thus, I am inclined to stick with the proposed version.

@andres-erbsen
Copy link
Contributor

@coqbot run full ci

@coqbot-app coqbot-app bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Dec 27, 2022
@haansn08
Copy link
Contributor Author

Maybe we rename the lemma filter_length_le and an equality condition filter_length_iff: length (filter l) = length l <-> forallb l?

@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Dec 27, 2022
theories/Lists/List.v Outdated Show resolved Hide resolved
theories/Lists/List.v Outdated Show resolved Hide resolved
@olaure01
Copy link
Contributor

Do we also want something like length (filter f l) + length (filter (fun x => negb (f x)) l) = length l?

For this, I would probably go through partition by adding something like:

Lemma partition_filter A f (l : list A) : partition f l = (filter f l, filter (fun x => negb (f x)) l).

(then you can use partition_length).

@olaure01 olaure01 added part: standard library The standard library stdlib. kind: enhancement Enhancement to an existing user-facing feature, tactic, etc. needs: changelog entry This should be documented in doc/changelog. labels Dec 27, 2022
@haansn08
Copy link
Contributor Author

@olaure01 Done. I also split up filter_length_iff as you suggested.

Copy link
Contributor

@andres-erbsen andres-erbsen left a comment

Choose a reason for hiding this comment

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

Just naming suggestions.

theories/Lists/List.v Show resolved Hide resolved
theories/Lists/List.v Outdated Show resolved Hide resolved
theories/Lists/List.v Show resolved Hide resolved
@andres-erbsen
Copy link
Contributor

Ok, looks good. Please add a changelog entry.

…lter_length, filter_length_le, filter_length_le
@haansn08
Copy link
Contributor Author

@andres-erbsen Ok, changelog entry added and commits squashed.

@andres-erbsen andres-erbsen removed the needs: changelog entry This should be documented in doc/changelog. label Dec 28, 2022
@andres-erbsen
Copy link
Contributor

@coqbot run full ci

I intend to merge if CI passes

@coqbot-app coqbot-app bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Dec 28, 2022
@andres-erbsen
Copy link
Contributor

@coqbot merge now

@coqbot-app
Copy link
Contributor

coqbot-app bot commented Dec 29, 2022

@andres-erbsen: You cannot merge this PR because:

  • No milestone has been set.

@andres-erbsen andres-erbsen added this to the 8.18+rc1 milestone Dec 29, 2022
@andres-erbsen
Copy link
Contributor

@coqbot merge now

@coqbot-app coqbot-app bot merged commit 41e76f5 into coq:master Dec 29, 2022
@haansn08 haansn08 deleted the filter_length branch December 29, 2022 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Enhancement to an existing user-facing feature, tactic, etc. part: standard library The standard library stdlib.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants