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

fewer convert methods for Missing and Nothing (take 2) #31602

Merged
merged 1 commit into from
Aug 13, 2019
Merged

fewer convert methods for Missing and Nothing (take 2) #31602

merged 1 commit into from
Aug 13, 2019

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Apr 3, 2019

And use typesubtract instead of subtyping to improve usability by handling undef sparams.

closes #30205

@vtjnash vtjnash requested a review from JeffBezanson April 3, 2019 20:25
@JeffBezanson
Copy link
Member

Do we need to add something to be able to infer typesubtract?

@ararslan ararslan added missing data Base.missing and related functionality types and dispatch Types, subtyping and method dispatch labels Apr 3, 2019
@vtjnash
Copy link
Member Author

vtjnash commented Apr 4, 2019

It apparently seems to be working ok already

@vtjnash
Copy link
Member Author

vtjnash commented Apr 15, 2019

Should we go with this, or at least bring up in triage it?

@JeffBezanson
Copy link
Member

Doesn't seem urgent. I would also like to understand how we're able to infer typesubtract...

@vtjnash
Copy link
Member Author

vtjnash commented May 9, 2019

Now that we've branched, can we land this on master and see how well it works out? I didn't intend for inference to handle typesubtract, but this happens to be one of the easiest cases for it anyways (the arguments end up predictably taking the form Union{typesubtract(Missing, Missing)==>Union{}, typesubtract(T, Missing)==>T}==>T)

JeffBezanson
JeffBezanson reviewed May 9, 2019
View reviewed changes
@vtjnash vtjnash force-pushed the jn/jb/convertmissingnothing branch from 1fb2e92 to 09be236 Compare May 9, 2019 23:27
nalimilan
nalimilan reviewed May 10, 2019
View reviewed changes
@JeffBezanson JeffBezanson mentioned this pull request May 28, 2019
Closed
vtjnash added a commit that referenced this pull request Jul 16, 2019
@vtjnash
gf-cache: avoid listing equivalent types
81240a4 
addresses and issue where we might be ambigous with other badly designed methods
such as the ambiguous Nothing/Missing rules (cf #31602)
vtjnash added a commit that referenced this pull request Jul 23, 2019
@vtjnash
gf-cache: avoid listing equivalent types
7cb0d2e 
addresses an issue where we might be find an ambiguity with badly designed methods,
such as the ambiguous Nothing/Missing rules (cf #31602)
vtjnash added a commit that referenced this pull request Aug 3, 2019
@vtjnash
gf-cache: avoid listing equivalent types
0c23e0c 
addresses an issue where we might be find an ambiguity with badly designed methods,
such as the ambiguous Nothing/Missing rules (cf #31602)
vtjnash added a commit that referenced this pull request Aug 3, 2019
@vtjnash
gf-cache: avoid listing equivalent types
e866576 
addresses an issue where we might be find an ambiguity with badly designed methods,
such as the ambiguous Nothing/Missing rules (cf #31602)
@JeffBezanson @vtjnash
fewer convert methods for Missing and Nothing
3888a6c 
And use `typesubtract` instead of subtyping
to improve usability by handling undef sparams.

Co-Authored-By: Jameson Nash <[email protected]>
@vtjnash vtjnash force-pushed the jn/jb/convertmissingnothing branch from 09be236 to 3888a6c Compare August 6, 2019 21:06
@vtjnash
Copy link
Member Author

vtjnash commented Aug 6, 2019

I'd like to land this before the feature freeze (although I suppose it should be non-breaking anyways), and give it some time to see how it works out in practice since it seems to need many fewer methods and just handles them with more ease.

@StefanKarpinski StefanKarpinski added this to the 1.3 milestone Aug 7, 2019
@vtjnash vtjnash mentioned this pull request Aug 8, 2019
Closed
@vtjnash vtjnash merged commit 71b63f3 into master Aug 13, 2019
@delete-merged-branch delete-merged-branch bot deleted the jn/jb/convertmissingnothing branch August 13, 2019 18:04
JeffBezanson
JeffBezanson reviewed Aug 13, 2019
View reviewed changes
base/essentials.jl
@@ -163,6 +163,7 @@ true
"""
function convert end

convert(::Type{Union{}}, x) = throw(MethodError(convert, (Union{}, x)))
Copy link
Member

Choose a reason for hiding this comment

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

Is this here to give a predictable ambiguity error instead of an ambiguity error involving two random methods? I suppose that's fine, just wondering if there is any other reason.

Copy link
Member Author

Choose a reason for hiding this comment

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

As you say, "predictability" :)

vtjnash added a commit that referenced this pull request Aug 16, 2019
@vtjnash
convert missing/nothing: fix previously undetected ambiguity in #31602
a32f974
vtjnash added a commit that referenced this pull request Aug 16, 2019
@vtjnash
gf-cache: avoid listing equivalent types
39c81e8 
addresses an issue where we might be find an ambiguity with badly designed methods,
such as the ambiguous Nothing/Missing rules (cf #31602)
@fredrikekre fredrikekre mentioned this pull request Aug 22, 2019
Closed
@vtjnash vtjnash mentioned this pull request Oct 15, 2019
Merged
@timholy timholy mentioned this pull request Apr 21, 2022
Closed
vtjnash added a commit that referenced this pull request Jul 11, 2022
@vtjnash
make convert(Union{},x) directly ambiguous
353ae21 
This should make it impossible to accidentally define or call this
method on foreign types.

Refs: #31602
Fixes: #45837
Closes: #45051
@vtjnash vtjnash mentioned this pull request Jul 11, 2022
Merged
KristofferC pushed a commit that referenced this pull request Jul 18, 2022
@vtjnash
make convert(Union{},x) directly ambiguous (#46000)
94e4082 
This should make it impossible to accidentally define or call this
method on foreign types.

Refs: #31602
Fixes: #45837
Closes: #45051
ffucci pushed a commit to ffucci/julia that referenced this pull request Aug 11, 2022
@vtjnash
make convert(Union{},x) directly ambiguous (JuliaLang#46000)
9c61dfe 
This should make it impossible to accidentally define or call this
method on foreign types.

Refs: JuliaLang#31602
Fixes: JuliaLang#45837
Closes: JuliaLang#45051
pcjentsch pushed a commit to pcjentsch/julia that referenced this pull request Aug 18, 2022
@vtjnash @pcjentsch
make convert(Union{},x) directly ambiguous (JuliaLang#46000)
35a3121 
This should make it impossible to accidentally define or call this
method on foreign types.

Refs: JuliaLang#31602
Fixes: JuliaLang#45837
Closes: JuliaLang#45051
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JeffBezanson JeffBezanson JeffBezanson left review comments

@nalimilan nalimilan nalimilan left review comments

Assignees
No one assigned
Labels
missing data Base.missing and related functionality types and dispatch Types, subtyping and method dispatch
Projects
None yet
Milestone
1.3
Development

Successfully merging this pull request may close these issues.
5 participants
@vtjnash @JeffBezanson @nalimilan @StefanKarpinski @ararslan