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.

Sign up for GitHub

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

Jump to bottom

Avoid type piracy in similar #823

Merged
merged 1 commit into from
Sep 16, 2020
Merged

Avoid type piracy in similar #823

merged 1 commit into from
Sep 16, 2020

Conversation

pabloferz
Copy link
Contributor

Here is one possible take on #821.

@pabloferz
Copy link
Contributor Author

I didn't check if it is because of this PR or something in julia master but the broken tests added in #810 are working here.

@KristofferC
Copy link
Contributor

KristofferC commented Sep 15, 2020 •
edited
Loading

Julia master reverted the change that caused those to break.

@pabloferz
Copy link
Contributor Author

pabloferz commented Sep 15, 2020 •
edited
Loading

Got it. Will leave those in case anything else changes again before the 1.6 RCs.

@mateuszbaran
Copy link
Collaborator

Thanks for working on this 👍 . This is more or less what I had in mind when I thought about possible solutions.

c42f
c42f reviewed Sep 16, 2020
View reviewed changes
Copy link
Member

@c42f c42f left a comment

Choose a reason for hiding this comment

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

This looks pretty good to me. We definitely have some type piracy here which should be addressed.

Would probably be a little neater to have a Union here.

src/abstractarray.jl Outdated Show resolved Hide resolved
@c42f c42f added the cleanup label Sep 16, 2020
@c42f c42f merged commit c7f01b6 into JuliaArrays:master Sep 16, 2020
@c42f
Copy link
Member

c42f commented Sep 16, 2020

Thanks very much!

@mateuszbaran mateuszbaran mentioned this pull request Sep 16, 2020
Closed
@pabloferz pabloferz deleted the pz/similar branch September 16, 2020 13:42
@mateuszbaran mateuszbaran mentioned this pull request Jul 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@c42f c42f c42f left review comments

Assignees
No one assigned
Labels
cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pabloferz @KristofferC @mateuszbaran @c42f