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

support InverseFunctions #622

Merged
merged 6 commits into from
May 11, 2023
Merged

support InverseFunctions #622

merged 6 commits into from
May 11, 2023

Conversation

aplavin
Copy link
Contributor

@aplavin aplavin commented Feb 25, 2023

Useful in, for example:

using Accessors

x = (a=1, b=1.23m)
y = @set x.b |> ustrip(m, _) = 2.34
# y == (a=1, b=2.34m)


using FlexiMaps

lr = maprange(@optic(log(ustrip(m, _))), 10cm, 10m, length=5)
# lr ≈ [0.1, √0.1, 1, √10, 10]m

Only works on 1.9+, no changes on earlier julias.

@codecov-commenter
Copy link

codecov-commenter commented Feb 25, 2023

Codecov Report

Merging #622 (c15b8cd) into master (bd09747) will not change coverage.
The diff coverage is n/a.

❗ Current head c15b8cd differs from pull request most recent head b79917b. Consider uploading reports for the commit b79917b to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff           @@
##           master     #622   +/-   ##
=======================================
  Coverage   89.06%   89.06%           
=======================================
  Files          16       16           
  Lines        1491     1491           
=======================================
  Hits         1328     1328           
  Misses        163      163           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Project.toml Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
@sostock
Copy link
Collaborator

sostock commented Feb 27, 2023

I would wait for the Julia 1.9.0 release before merging this, just in case something about the "package extension interface" is still changed before the release.

@aplavin
Copy link
Contributor Author

aplavin commented Feb 27, 2023

Well, then I won't insist, but note there are many very different packages that use extensions already: https://juliahub.com/ui/Search?q=extensions&type=code&f=Project.toml.

@aplavin
Copy link
Contributor Author

aplavin commented May 8, 2023

As expected, package extensions didn't change in the actual 1.9 release, and this PR code does not change either.
Can it be merged now?

@aplavin
Copy link
Contributor Author

aplavin commented May 8, 2023

Test failure seems to be an Aqua.jl bug: actually running ]resolve doesn't change Project.toml content.

@sostock
Copy link
Collaborator

sostock commented May 9, 2023

Test failure seems to be an Aqua.jl bug: actually running ]resolve doesn't change Project.toml content.

This is JuliaTesting/Aqua.jl#105. It looks like Aqua wants the sections ordered alphabeticallydifferently and the entries within the sections ordered alphabetically as well, which is not the order that Pkg uses.

Edit: We could just change the order to the one Aqua prefers, Pkg.resolve doesn’t seem to change it back.

aplavin and others added 3 commits May 10, 2023 00:12
Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>
@sostock sostock merged commit 3ab02d8 into PainterQubits:master May 11, 2023
13 checks passed
@aplavin
Copy link
Contributor Author

aplavin commented May 11, 2023

@sostock could you please register the version as well?

@sostock
Copy link
Collaborator

sostock commented May 11, 2023

Done: JuliaRegistries/General#83370

@oschulz
Copy link
Contributor

oschulz commented May 11, 2023

Regarding the limitation to v1.9 - why not making InverseFunctions a required depencency on <=v1.8? InverseFunctions is extremely lightweight - even on v1.9, using an extension probably won't save even half a millisecond of load time.

@aplavin
Copy link
Contributor Author

aplavin commented May 11, 2023

I just went for the 1.9-only approach because it's easiest (implement + convince to accept), that's the only reason.

@oschulz
Copy link
Contributor

oschulz commented May 12, 2023

Ok if I do a PR to extend it to <v1.9?

@oschulz
Copy link
Contributor

oschulz commented May 26, 2023

See #652

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