8000 Added read and write TSV with headers. Closes #194 by vsmalladi · Pull Request #627 · openwdl/wdl · GitHub
[go: up one dir, main page]
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

Added read and write TSV with headers. Closes #194 #627

Merged
merged 6 commits into from
Apr 1, 2024
Merged

Added read and write TSV with headers. Closes #194 #627

merged 6 commits into from
Apr 1, 2024

Conversation

vsmalladi
Copy link
Contributor
@vsmalladi vsmalladi commented Mar 29, 2024 •
edited
Loading

Checklist

  • Pull request details were added to CHANGELOG.md
  • Valid examples WDL's were added or updated to the SPEC.md (see the guide on writing markdown tests)

@wleepang
Copy link
Contributor

I think this PR needs a rebase. There are a lot of changes in the diff unrelated to read/write of TSV that are coming from other PRs.

@jdidion jdidion changed the base branch from wdl-1.1 to wdl-1.2 March 29, 2024 23:48
jdidion
jdidion requested changes Mar 29, 2024
View reviewed changes
SPEC.md Outdated
@@ -7951,18 +7951,26 @@ And `/local/fs/tmp/array.txt` would contain:
### `read_tsv`

```
Array[Array[String]] read_tsv(File)
Array[Array[String]] read_tsv(File), Array[Object] read_tsv(File, true), Array[Object] read_tsv(File, Boolean, Array[String])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please put each signature on a different line, like how we've done it for the other overloaded functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

SPEC.md Outdated
@@ -8010,14 +8070,31 @@ Example output:
### `write_tsv`

```
File write_tsv(Array[Array[String]])
File write_tsv(Array[Array[String]]|Array[Struct]), File write_tsv(Array[Array[String]], true, Array[String]), File write_tsv(Array[Struct], Boolean, Array[String])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

8000

SPEC.md Outdated
@@ -8405,6 +8511,19 @@ CODE

And `/local/fs/tmp/map.json` would contain:

`first\nsecond\nthird`

## File write_tsv(Array[Array[String]]|Array[Struct]), File write_tsv(Array[Array[String]], true, Array[String]), File write_tsv(Array[Struct], Boolean, Array[String])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like there might have been a merge issue here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@vsmalladi vsmalladi requested a review from jdidion March 30, 2024 05:16
jdidion
jdidion approved these changes Apr 1, 2024
View reviewed changes
Copy link
Collaborator
@jdidion jdidion left a comment

Choose a reason for hiding this comment

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

lgtm!

@vsmalladi vsmalladi merged commit 0bdce02 into wdl-1.2 Apr 1, 2024
@vsmalladi vsmalladi deleted the 194-read-tsv branch April 1, 2024 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdidion jdidion jdidion approved these changes

@mlin mlin Awaiting requested review from mlin

@wleepang wleepang Awaiting requested review from wleepang

@geoffjentry geoffjentry Awaiting requested review from geoffjentry

@patmagee patmagee Awaiting requested review from pa 5E2E tmagee

@cjllanwarne cjllanwarne Awaiting requested review from cjllanwarne

@markjschreiber markjschreiber Awaiting requested review from markjschreiber

@rhpvorderman rhpvorderman Awaiting requested review from rhpvorderman

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

Successfully merging this pull request may close these issues.
3 participants
@vsmalladi @wleepang @jdidion
0