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

[Merged by Bors] - chore(Init): add deprecation dates; remove long-deprecated items #12347

Closed
wants to merge 4 commits into from

Conversation

grunweg
Copy link
Collaborator

@grunweg grunweg commented Apr 22, 2024

All removed items are virtually unused in mathlib and have been deprecated for over a year.


Each commit can be reviewed individually; note that deprecation dates are first added
for all lemmas; and subsequently some deprecated lemmas are removed.

This change overlaps with #12350 (which removes a bunch of deprecated items about lists); this PR can go in regardless.


Open in Gitpod

 Bitte geben Sie eine Commit-Beschreibung für Ihre Änderungen ein. Zeilen,
trans_rel_left was used slightly, but there were easy to rewrite.
@grunweg grunweg added the awaiting-review The author would like community review of the PR label Apr 22, 2024
@grunweg
Copy link
Collaborator Author

grunweg commented Apr 22, 2024

@YaelDillies I hope this didn't create duplicate work. In any case, I guess you may find this interesting.

def nthLe (l : List α) (n) (h : n < l.length) : α := get l ⟨n, h⟩
#align list.nth_le List.nthLe

set_option linter.deprecated false in
@[deprecated]
@[deprecated] -- 2023-01-05
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think anything that's been deprecated for > 6 months is fair game to remove

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't sure if this one was still used. (There are many uses of deprecated lists stuff still.) Let me check...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It turns out this one can be removed; I've split these into #12350.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(That PR is now ready for review :-))

Comment on lines -123 to +106
@[deprecated and_comm] theorem and_comm' (a b) : a ∧ b ↔ b ∧ a := and_comm
#align and.comm and_comm
#align and_comm and_comm'
#align and_comm and_comm

@[deprecated and_assoc] theorem and_assoc' (a b) : (a ∧ b) ∧ c ↔ a ∧ (b ∧ c) := and_assoc
#align and_assoc and_assoc'
#align and_assoc and_assoc
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these two you should keep simply because they help teach people how the deprecation system works. Certainly this is how I learned about it.

Maybe @digama0 has a different opinion, though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In general, I'd rather have documentation in more discoverable places (than some undiscoverable corner of mathlib). I'm not sure if such a standard place already exists, though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nono, this is not about documentation. This is about the fact that ported code often contains and_comm' or and_assoc' and therefore the people porting the code are confronted with an (easy) task of undeprecation which teaches them how it works.

Copy link
Member

Choose a reason for hiding this comment

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

We're going to need to do something special if we want specific deprecations to last past the cutoff date.

Copy link
Collaborator

Choose a reason for hiding this comment

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

To be fair, I'm not sure how much more code we're going to port. So maybe this is moot.

Copy link
Collaborator Author

@grunweg grunweg Apr 23, 2024

Choose a reason for hiding this comment

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

💡 Could the porting wiki be a place to mention this? After all, this is where I'd look if I were to port code and want advice.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure yeah, I'm happy with that!

@fpvandoorn
Copy link
Member

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Apr 23, 2024
mathlib-bors bot pushed a commit that referenced this pull request Apr 24, 2024
)

All removed items are virtually unused in mathlib and have been deprecated for over a year.
@mathlib-bors
Copy link

mathlib-bors bot commented Apr 24, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore(Init): add deprecation dates; remove long-deprecated items [Merged by Bors] - chore(Init): add deprecation dates; remove long-deprecated items Apr 24, 2024
@mathlib-bors mathlib-bors bot closed this Apr 24, 2024
@mathlib-bors mathlib-bors bot deleted the MR-deprecation-remove-init branch April 24, 2024 01:21
Jun2M pushed a commit that referenced this pull request Apr 24, 2024
)

All removed items are virtually unused in mathlib and have been deprecated for over a year.
callesonne pushed a commit that referenced this pull request May 16, 2024
)

All removed items are virtually unused in mathlib and have been deprecated for over a year.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants