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

use StaticArraysCore #22

Merged
merged 3 commits into from
Sep 23, 2022
Merged

use StaticArraysCore #22

merged 3 commits into from
Sep 23, 2022

Conversation

mcabbott
Copy link
Member

@mcabbott mcabbott commented Sep 17, 2022

Replaces StaticArrays with StaticArraysCore. Needed for JuliaDiff/ForwardDiff.jl#599

This means dropping Julia < 1.6.

There is a bit of a hack to get around the fact that Size isn't available. Looks like this is well-tested.

@codecov-commenter
Copy link

Codecov Report

Merging #22 (96b91ee) into master (89f5a06) will increase coverage by 0.35%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #22      +/-   ##
==========================================
+ Coverage   86.48%   86.84%   +0.35%     
==========================================
  Files           1        1              
  Lines          74       76       +2     
==========================================
+ Hits           64       66       +2     
  Misses         10       10              
Impacted Files Coverage Δ
src/DiffResults.jl 86.84% <100.00%> (+0.35%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@devmotion
Copy link
Member

There is a bit of a hack to get around the fact that Size isn't available.

Maybe this should be fixed upstream and one should open issue in StaticArraysCore? It seems the docstring for similar_type (in StaticArraysCore) even mentions Size.

@mcabbott
Copy link
Member Author

That would certainly minimise the diff here. But (1) that means a chain of 4 PRs to get ForwardDiff free, and (2) from a quick look it wasn't just a few lines to move Size. Maybe this package can be adjusted later, which is why I left the comments here.

In the meantime, are there any actual downsides to this approach? Simple tests seem so say vec compiles away.

@devmotion
Copy link
Member

I opened an issue (JuliaArrays/StaticArraysCore.jl#12), so let's maybe see at least what the maintainers of StaticArraysCore think.

@devmotion
Copy link
Member

Apparently the plan is to move Size to StaticArraysCore within the next few days, so I guess it would be good to wait with this PR until that's done.

@devmotion
Copy link
Member

Size is now part of StaticArraysCore 1.4.0 🙂

src/DiffResults.jl Outdated Show resolved Hide resolved
Project.toml Outdated Show resolved Hide resolved
Project.toml Outdated Show resolved Hide resolved
src/DiffResults.jl Outdated Show resolved Hide resolved
src/DiffResults.jl Outdated Show resolved Hide resolved
src/DiffResults.jl Outdated Show resolved Hide resolved
mcabbott and others added 2 commits September 21, 2022 19:52
Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
Copy link
Member

@devmotion devmotion left a comment

Choose a reason for hiding this comment

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

LGTM

@mcabbott mcabbott merged commit 276d1d7 into JuliaDiff:master Sep 23, 2022
@mcabbott mcabbott deleted the static branch September 23, 2022 14:31
KristofferC pushed a commit to KristofferC/DiffResults.jl that referenced this pull request Dec 29, 2022
This reverts commit 276d1d7, reversing
changes made to 89f5a06.
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.

3 participants