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

Static type calculation for read_tsv isn't possible given the current overloads #690

Open
peterhuene opened this issue Nov 15, 2024 · 1 comment

Comments

@peterhuene
Copy link
Contributor

peterhuene commented Nov 15, 2024

As of 1.2, read_tsv has the following overloads as covered by the "variants" section in the function's description:

  • Array[Array[String]] read_tsv(File, [false])
  • Array[Object] read_tsv(File, true)
  • Array[Object] read_tsv(File, Boolean, Array[String])

The first two overloads are problematic as they're specified in terms of a value and not a type.

An example of read_tsv(file, header) where file is of type File and header is type Boolean demonstrates the problem; we're unable to statically determine the resulting type of this expression (is it Array[Array[String]] or Array[Object]?). That is to say, called in this fashion, the function is overloaded in its return type only which WDL has no facility to support.

Currently, the static analyzer used by sprocket defines read_tsv with these overloads instead:

  • Array[Array[String]] read_tsv(File)
  • Array[Object] read_tsv(File, Boolean) (if the second argument is false, it would be a runtime error)
  • Array[Object] read_tsv(File, Boolean, Array[String])

Removing the optional argument to the first overload means that you can't call read_tsv(file, false) to get a resulting Array[Array[String]], but as the optional (and I would argue, unnecessary) false argument was added in 1.2, I doubt anyone has done this yet.

Ideally I'd like to see these overloads split apart into two different functions: read_tsv and read_tsv_with_header.

read_tsv (doesn't read a header):

  • Array[Array[String]] read_tsv(File) (compatible with WDL 1.0+)
  • Array[Object] read_tsv(File, Array[String]))

read_tsv_with_header:

  • Array[Object] read_tsv_with_header(File, [Array[String]])
@peterhuene
Copy link
Contributor Author

peterhuene commented Nov 15, 2024

Related:

I think the overloads of write_tsv could also be simplified to:

  • File write_tsv(Array[Array[String]], [Array[String]]) (if the second argument is present, the file will have the specified header; if not, no header is written)
  • File write_tsv(Array[Struct], [Boolean, [Array[String]]])

Unlike read_tsv, though, the first argument to the write_tsv overloads differ and therefore there's no ambiguity for a static type calculation.

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

No branches or pull requests

1 participant