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

Mult dim reverse #31716

Closed
wants to merge 12 commits into from
Closed

Mult dim reverse #31716

wants to merge 12 commits into from

Conversation

HDictus
Copy link

@HDictus HDictus commented Apr 13, 2019

for #29871

test/arrayops.jl Outdated Show resolved Hide resolved
@HDictus
Copy link
Author

HDictus commented Apr 15, 2019

I noticed something while working on this, which I think is related to #9498
There exists a method reverse(::Array; dims=::Integer).
I created a method reverse(::AbstractArray; dims)

If I call reverse(somearray, dims=sometuple) it presently seems to attempt to call reverse(::Array; dims=::Integer), for which types do not match, rather than reverse(::AbstractArray; dims) for which types do match.

My solution for now is to rename conflicting methods _reverse, so that they will be "private" and called by reverse(::AbstractArray; dims) when dims is scalar. These should probably be renamed back if #9498 is ever fixed.

As a side note, I noticed that there is code duplication between different reverse/_reverse methods. It's not a big deal I think, but it'd make sense to abstract the common components to something.

@HDictus
Copy link
Author

HDictus commented Apr 16, 2019

Hm, I don't see how those two build problems could relate to base. Can anyone point me where to start to make these checks pass?

@HDictus
Copy link
Author

HDictus commented Apr 21, 2019

I can't see how my changes could cause either of these errors for win32 build (rm: cannot remove 'share/julia/test': Device or resource busy)
(command timed out: 3600 seconds without output running ['bin/julia.exe', '-e', 'Base.runtests(["all"]; ncores=min(Sys.CPU_THREADS, 8))'], attempting to kill) or xcode test (Code:ERROR, Class:Net, curl error: Could not resolve host: github.com). I'd really like to merge this though, it's needed for an issue in DSP. Is it possible there was a temporary problem with the server that runs the CI? If that's possible, could someone run again?

@musm
Copy link
Contributor

musm commented Dec 13, 2020

Addressed by #37367

@musm musm closed this Dec 13, 2020
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.

4 participants