-
Notifications
You must be signed in to change notification settings - Fork 434
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
Implement bash translator #674
Conversation
Alas GH doesn't support attaching *.ipynb, so I have had to rename it *.txt, but here's the test notebook. The following params.yml can be used with this:
|
A friendly ping to the nteract team about this PR. All tests pass locally, and this is currently blocking us from using papermill in our analyses. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, @kdm9! Can you please add some tests to test_translators.py
.
@rohitsanj tests implemented and passing. There are some unrelated failures on my machine, all to do with the google cloud interaction, which I assume is because I don't have a GCS environment correctly configured. Any chance you can allow the CI to run on this PR, so we can see if all is green in a well-configured environment? |
Codecov Report
@@ Coverage Diff @@
## main #674 +/- ##
==========================================
+ Coverage 91.72% 91.86% +0.13%
==========================================
Files 17 17
Lines 1583 1622 +39
==========================================
+ Hits 1452 1490 +38
- Misses 131 132 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding tests! They passed on the CI chain so no worries about local dev issues there
Support translation of parameters to bash/*NIX shell
Thanks for papermill! I have used it for python notebooks, but a lot of my work is best done in shell, for which I use jupyter notebooks with the bash_kernel (via *.sh scripts and jupytext).
Alas papermill doesn't seem to support bash/shell script, due to a hitherto unimplemented bash translator. This PR implements a
BashTranslator
, and therefore enables bash kernel notebooks to be parametrised. This code supportsstr
,list
,bool
1,None
,int
, andfloat
parameter values. Dict could in theory be supported via associative arrays but for now they are not implemented2.For a functioning example, please see the attached notebook.
Best,
K
Footnotes
Bash doesn't really have a bool type, but
true
andfalse
are at least common and sensible string constants for truthy-ness and falsy-ness. See for example this SO answer. ↩Associative arrays require quite recent bash versions, and I've never seen them used outside tutorials. Supporting them would also likely require a more involved code path as one must pre-declare an associative array in bash, and then declare one entry per line (see link above). ↩