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

Clarify default values: Position vs Keyword #32020

Closed
wants to merge 19 commits into from

Conversation

taqtiqa-mark
Copy link

Tries to address my confusion and that of David Klaffenbach here:
https://discourse.julialang.org/t/define-f-with-keyword-arguments-function-f-does-not-accept-keyword-arguments/24186

Maybe others also too sheepish to speak-up publicly?

wsmoses and others added 2 commits May 7, 2019 14:35
@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented May 13, 2019

Just linking that any resolution of issue #9498 will likely require this section of the docs to be updated.
Maybe a better example to put into the doc is that given by from Seth Bromberger

Copy link
Member

@StefanKarpinski StefanKarpinski left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification. The level of alarm seems somewhat overstated. Could you tone it down a bit?

Fix compilation with external LLVM
Added an example of issue JuliaLang#9498 - since it is not scheduled to be fixed until Julia 2.0.
Kept the example of ambiguous/last-seen default code being run because that is the core
use case for default values.
@KristofferC
Copy link
Member

KristofferC commented May 14, 2019

I am still not sure what the confusion in https://discourse.julialang.org/t/define-f-with-keyword-arguments-function-f-does-not-accept-keyword-arguments/24186 actually is and how the manual does not explain it. Is it #9498? In that case I suggest just adding a short sentence to that issue.

If it is something else, it would be good to pinpoint exactly what the manual fails to convey and try to more "surgically" insert that into the text.

@StefanKarpinski
Copy link
Member

It seems that most of the confusion in that thread came from reusing the same REPL session in which other methods of f had already been defined. In particular, this example:

julia> f(a=1,b=2)=5
f (generic function with 3 methods)
	
julia> f(;a=1,b=2)=10
f (generic function with 3 methods)

is given here as if this was some weird or surprising aspect of keyword/positional arguments, when it is just an artifact of defining the zero-argument method of f twice—when a method is redefined, the last version defined takes precedence. This could be called out explicitly, but I don't think it's appropriate that it be the very first thing that a new user is confronted with when being introduced to optional argument syntax. I also don't think that bringing in the relatively obscure #9498 at the very beginning and then belaboring it so much is necessary. Mentioning both at the very end of the section on keyword arguments seems sufficient.

@StefanKarpinski
Copy link
Member

The only real connection between the issue of REPL session reusage and default arguments is that defining methods with defaults is a particularly good way to redefine a previously defined method in an obscure way and confuse yourself. That is probably enough of a common hazard to warrant mention here, but it's not specific to method definitions or default argument values—it's something to be aware of in general when using the REPL.

stevengj and others added 6 commits May 14, 2019 09:09
The tiers of support are now maintained in one place, i.e. on the downloads page on the website. It was difficult to have an accurate list of tiers for master, and much simpler to have support tiers for the latest stable release instead. Thus, the downloads page is the right place to host this information and curate it.
@taqtiqa-mark
Copy link
Author

From my PoV the REPL issue was not the source of confusion, fact it served to signal something out of the ordinary was going on.

Issues with the current document:

  1. Does not deal with keyword arguments, despite the prominent heading (the subtle technical aside about dispatch at the end makes it sound like the above was about keyword args).
  2. Examples should be given for both approaches to setting default values to make clear what is different and where the subtle bugs can creep in. The case where a function has defaults set using positional and keyword arguments deserves to be called out as special (as Julia user base increases DSL's will be built and it is not unknown to try to cater to users from both schools of thought/preference (positional vs keyword preferences))
  3. f() being silently redefined for me and f() not raising an ambiguous method error broke my mental model of Julia.... at some meta/deeper level I thought Julia was dealing with f(PositionalArgDefaults(....)) and f(KeywordArgDefaults(....)) and I expected an ambiguous error in response to f(). Now my PoLS expectations now would be f() and f(;) are the way to get the two different sets of default definitions.... Ack that may be wrongheaded, and I'll come to regret writing it ;) Anyway if you check you'll see that f(;) returns the keyword default value, and then... even it gets clobbered by the positional defaults definition - there is no escape... if you check a positional default definition in a new REPL you'll see that f(;) returns the positional default even though no keyword arguments have been used (is this Keyword arguments affect methods dispatch #9498 or a distinct issue?).
  4. Regarding Keyword arguments affect methods dispatch #9498 and new users: I think you scare them off or burn their goodwill by flagging that there is a issue about some behavior without giving them the opportunity to fully grasp what it is. Those interested are saved the time and grief of having to figure it out for themselves and then decide (with their incomplete understanding) if it is worth the risk of using the functionality. Those not interested will skip the detail. All benefit no cost, in my opinion.

I agree with @StefanKarpinski #9498 must be mentioned. It will give more understanding and not less, lead to fewer issues and not more - but will leave this to you.

I agree with @StefanKarpinski that the downside of positional defaults needs to be made explicit:

defining methods with defaults is a particularly good way to redefine a previously defined method in an obscure way and confuse yourself

What is the Julian recommended way to handle default values?
If that exists then we should link to that part of the documentation.

My 2c: It seems to me, at this point, that what I would have appreciated is something like the following take away/impression (until 2.0):

  1. Handle default values using as many Types and with as many inner or outer constructors as you need to.
  2. Realy, try again, take another course, read another book, another blog post, use all the power and performance of Types and their constructors.
  3. Seriously, ask around, stop people in the street.... try to use Types and their constructors to handle your default values.
  4. If that cannot work, then try to use keyword arguments as sparingly as possible and bear in mind Keyword arguments affect methods dispatch #9498, and invoke the default method using f(;) syntax/style to make clear there are keyword defaults being used.
  5. If that cannot work, then try to use positional defaults as sparingly as possible and bear in mind the extra methods spawned, and make sure you write test cases for each of those method definitions that don't exist in your code base, and bear mind your code coverage tools won't guard you against un-tested behavior creeping in throughout your business - and when some regulator or inquiry flags you.... don't say you were not warned :)
  6. If those cannot work and you must freely use both positional defaults and keyword defaults in your coding.... good-luck ;)

taqtiqa-mark and others added 6 commits May 15, 2019 10:08
Tries to address my confusion and that of David Klaffenbach here:
https://discourse.julialang.org/t/define-f-with-keyword-arguments-function-f-does-not-accept-keyword-arguments/24186

Maybe others also too sheepish to speak-up publicly?
Added an example of issue JuliaLang#9498 - since it is not scheduled to be fixed until Julia 2.0.
Kept the example of ambiguous/last-seen default code being run because that is the core
use case for default values.
@taqtiqa-mark
Copy link
Author

Hmm, editing in github then locally is getting messy.
Also this issue makes me think the whole issue of defaults is not trivial.
#10146

May retry this if I think I have some concrete insight that adds value.

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.

10 participants