Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

refactor(*): remove unnecessary uses of λ #3935

Closed
wants to merge 7 commits into from
Closed

Conversation

fpvandoorn
Copy link
Member

Most changes are of the form (λ x, x + c) -> (+ c) and (λ x, c + x) -> ((+) c) (and also for *, /, <, , etc)
It doesn't work for -, since neither (- x) and ((-) x) are parser correctly (presumably because of the unary minus
The shorter version also sometimes doesn't work if coercions are inserted.


the expression (- x) is interpreted as the unary minus, instead of a partially applied binary minus
@fpvandoorn fpvandoorn added the awaiting-review The author would like community review of the PR label Aug 24, 2020
Copy link
Collaborator

@jalex-stark jalex-stark left a comment

Choose a reason for hiding this comment

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

LGTM.

In addition to the changes removing redundant lambdas, there are some changes that change whitespace, add/remove parens, and shorten expressions with dot notation (see e.g. src/tactic/omega/coeffs.lean). They all seem uncontroversial.

This touches many files, so would be good to get merged quickly.

∧ (∀ (m : ℕ), (m : with_top ℕ) < n →
differentiable_on 𝕜 (λ x, iterated_fderiv_within 𝕜 m f s x) s) :=
differentiable_on 𝕜 ((iterated_fderiv_within 𝕜 m f s)) s) :=
Copy link
Collaborator

Choose a reason for hiding this comment

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

are the extra parens necessary? (I'm guessing they're autogenerated?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

(the same comment applies to the next few lines of the same file. I didn't see potentially-redundant parentheses anywhere else.)

@bryangingechen
Copy link
Collaborator

I'm sure I can eventually get used to this, but this surely hurts readability for the "curious mathematician". The difference between (+ c) and ((+) c) will definitely take some time for me to internalize. Maybe we should collect some opinions on Zulip?

@kim-em
Copy link
Collaborator

kim-em commented Aug 25, 2020

Why are we doing this?

@fpvandoorn
Copy link
Member Author

@robertylewis
Copy link
Member

I'm also not sure about all of these changes. Some of the eta reductions look reasonable. But when infix notation is involved, the lambda version seems much clearer. Not only for beginners, even for me. Is there an advantage to the change that I'm missing?

@robertylewis robertylewis added the RFC Request for comment label Aug 25, 2020
@fpvandoorn fpvandoorn removed the awaiting-review The author would like community review of the PR label Aug 25, 2020
@fpvandoorn
Copy link
Member Author

fpvandoorn commented Aug 25, 2020

Ok, that's fair. I'll close this PR. If we consistently want to use a certain notation, I might open a new PR.

@fpvandoorn fpvandoorn closed this Aug 25, 2020
@fpvandoorn fpvandoorn deleted the lambdas branch December 29, 2020 09:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
RFC Request for comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants