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

Changed yaml parsing to preserve types in notebook. Made multiple yaml inputs valid. #88

Closed
wants to merge 4 commits into from

Conversation

MSeal
Copy link
Member

@MSeal MSeal commented Jan 14, 2018

Yaml inputs had to be composed before calling papermill when there didn't seem to be a strict reason to limit inputs. This change makes it so you can mix and match the command inputs to fit your incoming parameter data. In my case I had multiple json objects as well as individual parameters being collected that needed separate assignments in the notebook. Now later yamls will override earlier ones with individual parameters taking highest preference.

Furthermore, passing a json object will give you a dict in your notebook instead of a string. This makes passing complex parameters very nice for consumption in the notebook.

Also added specs for R parameter decoding (and validated they worked even though there aren't R kernel automated specs yet in the repo).

@MSeal
Copy link
Member Author

MSeal commented Jan 14, 2018

The R calls corresponding to the R decode specs:

#R version 3.3.2 

library("rjson")

t0 = "foo"
t1 = "{\"foo\": \"bar\"}"
t2 <- fromJSON(t1)

print(t0)
print(t1)
print(t2)

t3 = list("foo" = "bar")
t4 = list("foo" = "\"bar\"")

print(t3)
print(t4)

t5 = list("foo" = list("bar"))
t6 = list("foo" = list("bar" = "baz"))
t7 = list("foo" = list("bar" = "\"baz\""))

print(t5)
print(t6)
print(t7)

t8 = list("foo")
t9 = list("foo", "\"bar\"")

print(t8)
print(t9)

t10 = list(list("foo" = "bar"))
t11 = list(list("foo" = "\"bar\""))

print(t10)
print(t11)

t12 = 12345
t13 = -54321

print(t12)
print(t13)

t14 = TRUE
t15 = FALSE

print(t14)
print(t15)

with outputs:

[1] "foo"
[1] "{\"foo\": \"bar\"}"
$foo
[1] "bar"

$foo
[1] "bar"

$foo
[1] "\"bar\""

$foo
$foo[[1]]
[1] "bar"


$foo
$foo$bar
[1] "baz"


$foo
$foo$bar
[1] "\"baz\""


[[1]]
[1] "foo"

[[1]]
[1] "foo"

[[2]]
[1] "\"bar\""

[[1]]
[[1]]$foo
[1] "bar"


[[1]]
[[1]]$foo
[1] "\"bar\""


[1] 12345
[1] -54321
[1] TRUE
[1] FALSE

This was referenced Jan 14, 2018
@willingc
Copy link
Member

@MSeal cool! Can you split this PR into two: one for CLI/multiple and one for R stuff? Will be easier to test, review, and merge. Thanks.

@MSeal
Copy link
Member Author

MSeal commented Jan 14, 2018

Sure I'll pull the R specs into another PR tonight

@codecov
Copy link

codecov bot commented Jan 14, 2018

Codecov Report

Merging #88 into master will increase coverage by 2.82%.
The diff coverage is 93.18%.

@@            Coverage Diff             @@
##           master      #88      +/-   ##
==========================================
+ Coverage   55.46%   58.28%   +2.82%     
==========================================
  Files           8        8              
  Lines         869      887      +18     
==========================================
+ Hits          482      517      +35     
+ Misses        387      370      -17

@MSeal
Copy link
Member Author

MSeal commented Jan 15, 2018

Closing this PR for the 3 broken down versions.

@MSeal MSeal closed this Jan 15, 2018
@MSeal MSeal deleted the preserveTypes branch January 15, 2018 06:21
ashutoshvarma added a commit to ashutoshvarma/papermill that referenced this pull request Jan 6, 2021
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.

None yet

2 participants