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

Question mark operator for unwrapping optional values #551

Open
jdidion opened this issue Apr 3, 2023 · 5 comments
Open

Question mark operator for unwrapping optional values #551

jdidion opened this issue Apr 3, 2023 · 5 comments

Comments

@jdidion
Copy link
Collaborator

jdidion commented Apr 3, 2023

This is purely syntactic sugar for select_first([x]) where x is a value that may be None.

Int? x = 5
Int x1 = x?
Int? y = None
Int y1 = y?  # error

If we add this operator then we can get away from special-casing optional behavior within string interpolations. I.e. "~{x + y}" would no longer be allowed (i.e. deprecated in 1.2 and disallowed in 2.0) and instead you'd have to write "~{x? + y?}", just as you would in an expression outside of an interpolation.

@DavyCats
Copy link
Contributor

DavyCats commented Apr 4, 2023

An alternative to select_first for "unoptionaling" (there's probably a better word for this...) variables sounds like a good idea to me. It always seemed a bit unintuitive that you had to use select_first for this.

If we add this operator then we can get away from special-casing optional behavior within string interpolations. I.e. "~{x + y}" would no longer be allowed (i.e. deprecated in 1.2 and disallowed in 2.0) and instead you'd have to write "~{x? + y?}", just as you would in an expression outside of an interpolation.

In command sections it is pretty common to add an optional to a String. As this will result in none if the optional is not defined, it's an easy (and compact) way of omitting options from a command if no value for it was provided in the inputs. I'm pretty sure this is the reason for this special-casing in the first place. I'm not sure that this new syntax would address this. If its behavior is the same as that of select_first, then ~{"--option " + x?} would error if x is none.

eg. currently this is possible:

  Int? optional_value
# [...]
  ~{"--optional-option " + optional_value}

The alternative (which would have to be used if the above gets deprecated) is a lot more verbose:

# current syntax
~{if defined(optional_value) then "--optional-option " + select_first([optional_value]) else ""}
# proposed syntax
~{if defined(optional_value) then "--optional-option " + optional_value? else ""}

@rhpvorderman
Copy link
Contributor

rhpvorderman commented Apr 4, 2023

Your strength is your weakness and your weakness is your strength. On one hand select_first([x]) is rather clunky, on the other hand it therefore is very explicit and in your face. YOU ARE ABOUT TO UNBOX THIS OPTIONAL.

The question mark is subtle, but I wonder if it isn't too subtle.

How about a function called select? It fits well with the already existing functions select_first and select_all so it will be nice and consistent.

Int? x = 5
Int x1 = x?
Int? y = None
Int y1 = select(y)  # error

Looking at the code example, that actually looks quite nice!

@jdidion
Copy link
Collaborator Author

jdidion commented Apr 4, 2023

@DavyCats I borrowed the "unwrap" terminology from Rust, which has an Option type with a function of that name with exactly the described behavior. Haskell has something similar. Scala and OCaml both have option types with a get function, which seems a little too generic.

When I talk about special casing, I'm referring to the two exceptions mentioned here: https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#coercion-of-optional-types. The behavior of "catching" exceptions in placeholders and converting them to None would be preserved. Your example would just need to be changed to ~{"--optional-option " + optional_value?}.

@rhpvorderman I'm somewhat ambivalent with a slight preference for the ? syntax. I think it has a nice symmetry with using ? to declare an optional type. Going from select_first([x]) to select(x) doesn't really save much typing or add much clarity. My personal bias is towards adding syntax when it helps to simplify very common operations, and unwrapping optionals is one of the most common in WDL (at least in my experience). That said, if everyone thinks select is more clear (or some other name like unwrap) and/or adding a library function rather than syntax is a better approach I'm happy to go that route.

@rhpvorderman
Copy link
Contributor

I think it has a nice symmetry with using ? to declare an optional type.

But declaration and unwrapping are different. So why use the same operator?

Going from select_first([x]) to select(x) doesn't really save much typing or add much clarity.

  • save much typing: I really don't care that much about the number of characters (unless it gets ridiculous). I don't write write-only workflows. Clarity is much more important than that.
  • clarity. I do agree that select is not very clear. But given that we already have tools like select_first and select_any I think to a seasoned WDL developer, it is clear. It is also nice to have consistency. select, select_first, select_any has this.

I think both ? and select have their merits and this is much a question of taste. Maybe someone else has a proposal that we both very much like?

@markjschreiber
Copy link

I like the use of ? and it will be familiar to some developers who have experience with languages with this operator. In most cases it will make code more readable than select_first([x]). If a function call is preferred then unwrap(x) does lean into the fact that in many languages Optional has two subtypes Some<x> and None where calling unwrap on the former gives you a value and an exception with the latter. Going to that level seems excessive for WDL, although it would allow the nice switch constructs you get with Scala and Rust.

So, I'd vote for ? and/ or select(T?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants