Tag Based Parameters

[sam2332]

Im using it and its wonderful, this version supports putting the tag "parameters" on a cell to select it.

[takluyver]

Hi Sam, is there any particular reason you closed this? I'm open to the idea of using a tag to indicate the parameters cell, I just haven't had time to work on nbparameterise (or a whole lot of other projects) recently.

[sam2332]

It needs some work on the implementation, I also want to make the current version ignore any cell that contains the word import during cell seeking. I will reopen with some changes

…

On Sat, Apr 9, 2022, 3:09 PM Thomas Kluyver ***@***.***> wrote: Hi Sam, is there any particular reason you closed this? I'm open to the idea of using a tag to indicate the parameters cell, I just haven't had time to work on nbparameterise (or a whole lot of other projects) recently. — Reply to this email directly, view it on GitHub <#23 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHG3VQNDUZNIJFZETJSCWTVEHIWZANCNFSM5PVIARLQ> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[codecov]

Codecov Report

Merging #23 (c821e33) into master (5f1767a) will decrease coverage by 2.77%.
The diff coverage is 72.72%.

❗ Current head c821e33 differs from pull request most recent head 4e864e3. Consider uploading reports for the commit 4e864e3 to get more accurate results

@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
- Coverage   92.08%   89.30%   -2.78%     
==========================================
  Files           3        3              
  Lines         139      159      +20     
==========================================
+ Hits          128      142      +14     
- Misses         11       17       +6     

Impacted Files	Coverage Δ
nbparameterise/code.py	88.31% <72.72%> (-6.43%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f1767a...4e864e3. Read the comment docs.

[takluyver]

Thanks!

I'd rather not skip cells with import in the code. I can see why you want that, but 'first code cell' is a nice easy rule to understand, and I don't want to make it more complex. The option you're adding to use a 'parameters' tag gives some flexibility, and I'm also working on replacing the parameters without discarding other code in the same cell (see PR #19), which would mean you could have your parameters and imports in the same cell if you wanted.

[takluyver]

Thanks - I've gone through and identified a few specific changes to make.

Could you also look at adding a test for using a tagged cell rather than the first code cell?

[sam2332]

Nice. I will make some changes today.

…

On Thu, Apr 14, 2022, 6:52 AM Thomas Kluyver ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In nbparameterise/code.py <#23 (comment)> : > + if 'metadata' in cell: + if 'tags' in cell['metadata']: + if any([i.lower()=="parameters" for i in cell['metadata']['tags']]): Oh, one more thought: rather than lots of if statements, it might be neater to do something like this: tags = cell.get('metadata', {}).get('tags', []) Up to you, though. 🙂 — Reply to this email directly, view it on GitHub <#23 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHG3VTIVYYAI7NPGUG3Q4LVE72HJANCNFSM5PVIARLQ> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[sam2332]

I believe this code is ready for review once again.

[sam2332]

please let me know if there is anything else needed on this

[takluyver]

Thanks, and sorry it took me so long to get to this. I was moving house around the time you were doing this, and then I forgot about it.

I've rebased your changes and made a few further ones as PR #27.