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

Fix deprecation warning #51

Merged
merged 3 commits into from
May 15, 2023
Merged

Fix deprecation warning #51

merged 3 commits into from
May 15, 2023

Conversation

jkrumbiegel
Copy link
Contributor

On Julia 1.7.2 with --depwarn=error this expression Vararg{<:AbstractArray} causes the error

ERROR: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
Stacktrace:
 [1] UnionAll(v::TypeVar, t::Any)
   @ Core ./boot.jl:255
 [2] top-level scope
   @ REPL[25]:1

On Julia 1.7.2 with `--depwarn=error` this expression `Vararg{<:AbstractArray}` causes the error
```julia
ERROR: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
Stacktrace:
 [1] UnionAll(v::TypeVar, t::Any)
   @ Core ./boot.jl:255
 [2] top-level scope
   @ REPL[25]:1
```
@codecov
Copy link

codecov bot commented May 16, 2022

Codecov Report

Merging #51 (efd929d) into master (9fbda81) will increase coverage by 0.60%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
+ Coverage   84.55%   85.15%   +0.60%     
==========================================
  Files           1        1              
  Lines         123      128       +5     
==========================================
+ Hits          104      109       +5     
  Misses         19       19              
Impacted Files Coverage Δ
src/MappedArrays.jl 85.15% <100.00%> (+0.60%) ⬆️

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

@johnnychen94 johnnychen94 self-assigned this May 16, 2022
@blegat
Copy link

blegat commented Dec 23, 2022

This is causing issues for HybridSystems as well: blegat/HybridSystems.jl#53
Could this be merged ?

@giordano
Copy link

giordano commented Feb 2, 2023

Bump. Pinging @timholy @johnnychen94

@timholy timholy closed this Feb 3, 2023
@timholy timholy reopened this Feb 3, 2023
@timholy
Copy link
Member

timholy commented Feb 3, 2023

Rerunning tests...

@juliohm
Copy link

juliohm commented May 14, 2023

I opened another issue recently #56 reporting this warning. It would be great if someone could merge this.

ping @timholy @johnnychen94

@juliohm
Copy link

juliohm commented May 15, 2023

Superseded by #57

Can someone please review and merge that PR?

@giordano giordano mentioned this pull request May 15, 2023
@giordano
Copy link

"Superseded" is an interesting term since that PR is identical to this one, hard to tell how that's any better.

@juliohm
Copy link

juliohm commented May 15, 2023

"Superseded" is an interesting term since that PR is identical to this one, hard to tell how that's any better.

Didn't check the contents of this one, but just meant that there was a more recent PR opened with a fix. Since this one here has failing tests, I assumed that is was out of date.

In any case, it would be super nice if someone with write access could merge this and release a patch 🙏🏽

@jkrumbiegel
Copy link
Contributor Author

I'll close and reopen to rerun the tests, I can't see the logs of the failures anymore as they are too old.

@jkrumbiegel jkrumbiegel reopened this May 15, 2023
@timholy
Copy link
Member

timholy commented May 15, 2023

I fixed the failures (locally), thanks for the PR! If this passes tests I will merge.

@timholy
Copy link
Member

timholy commented May 15, 2023

(Weird. It looks like the date on my WSL instance does not match that of the underlying Windows machine.)

@timholy
Copy link
Member

timholy commented May 15, 2023

The test failure is #43 (comment). That's clearly worthy of a separate PR, so I'll merge this.

@timholy timholy merged commit 9c2c5b1 into JuliaArrays:master May 15, 2023
8 of 14 checks passed
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

6 participants