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

chore(data/finset/image): reduce imports #17886

Closed
wants to merge 6 commits into from
Closed

Conversation

negiizhao
Copy link
Collaborator

@negiizhao negiizhao commented Dec 10, 2022

RFC. It's not important. Just seems like it shouldn't depend on data.int.order.basic.

Maybe int.mod_nonneg and int.mod_lt_of_pos should be moved to a more basic file?

Zulip


Open in Gitpod

@negiizhao negiizhao added awaiting-review The author would like community review of the PR awaiting-CI The author would like to see what CI has to say before doing more work. labels Dec 10, 2022
@@ -405,16 +407,18 @@ end

lemma mem_range_iff_mem_finset_range_of_mod_eq [decidable_eq α] {f : ℤ → α} {a : α} {n : ℕ}
(hn : 0 < n) (h : ∀ i, f (i % n) = f i) :
a ∈ set.range f ↔ a ∈ (finset.range n).image (λi, f i) :=
suffices (∃ i, f (i % n) = a) ↔ ∃ i, i < n ∧ f ↑i = a, by simpa [h],
Copy link
Member

Choose a reason for hiding this comment

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

Does the old proof no longer work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Old proof needs some lemmas in data.int.order.basic.

Copy link
Member

Choose a reason for hiding this comment

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

Which ones?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

int.mod_nonneg and int.mod_lt_of_pos.

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 a little wary of this this split; those seem like pretty fundamental properties of mod, so I would prefer not to have to effectively reprove them to prove this lemma.

Can you split this into two PRs; one that moves the imports, and one that changes int.order.basic to int.basic? edit: thanks!

@github-actions github-actions bot removed the awaiting-CI The author would like to see what CI has to say before doing more work. label Dec 10, 2022
@eric-wieser eric-wieser added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Dec 10, 2022
@negiizhao negiizhao added awaiting-review The author would like community review of the PR blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. and removed awaiting-author A reviewer has asked the author a question or requested changes labels Dec 10, 2022
@mathlib-dependent-issues-bot mathlib-dependent-issues-bot removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Dec 10, 2022
@mathlib-dependent-issues-bot
Copy link
Collaborator

This PR/issue depends on:

@eric-wieser eric-wieser changed the title chore(data/finset): reduce imports chore(data/finset/image): reduce imports Dec 10, 2022
@YaelDillies
Copy link
Collaborator

That doesn't really seem important. data.int.order.basic is quite a basic file, and we're just proving some results about % and finset.image.

@negiizhao negiizhao closed this Feb 18, 2023
@YaelDillies YaelDillies deleted the FR_finset_import branch February 18, 2023 08:37
@YaelDillies YaelDillies removed the awaiting-review The author would like community review of the PR label Feb 27, 2023
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

4 participants