Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 182 additions & 0 deletions jupyter/Scale Output Example.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
{
Copy link
Author

Choose a reason for hiding this comment

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

@jjaco2 You probably want to review the strings in this file. They may be over explanatory.

Copy link

Choose a reason for hiding this comment

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

See comment below on this -- I think this is an appropriate level of detail. We tend to under-explain on code examples, and I like opportunities to slowly change that.

"cells": [
{
"cell_type": "markdown",
"metadata": {},
"source": [
"# Simple Scalar Output Example\n",
"\n",
"In this example, we will define an integer parameter called `Integer input` and will output a floating point called `My scalar output` that can be bound to a numeric dashboard tile."
Copy link

Choose a reason for hiding this comment

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

Maybe reword this slightly to be clearer about what this example helps the user do:

'This example shows how to create a scalar output [in Jupyter? not sure if that makes sense] that you can bind to a numeric dashboard tile in SystemLink.'

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we standardize on example variable naming here? E.g. using "My integer input" or "Scalar output" instead?

]
},
{
"cell_type": "code",
"execution_count": 1,
"metadata": {},
"outputs": [],
"source": [
"import scrapbook as sb \n",
"from random import *"
Copy link
Contributor

Choose a reason for hiding this comment

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

import * is often discouraged in favor of either importing the module directly, or importing what you need from the module, like import random followed by random.uniform() or from random import uniform followed by uniform(). https://ni.github.io/python-styleguide/#O-1-6

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd also recommend moving the random import above the scrapbook import and adding a blank line between the two, since random is a standard library import which is typically lifted above 3rd party imports. https://ni.github.io/python-styleguide/#O-1-3

]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"## Parameters\n",
"\n",
"The parameters block is where you define all of the inputs and outputs to the notebook. If you look at __Property Inspector >> Advanced Tools >> Cell Metadata__, while the `# Parameters` code cell block is selected, you will see the json that describes the inputs and outputs. Note that when you edit that json, you must click the small check mark under \"Cell Metadata\" to save the changes. In this example you can see we have configured a numeric input and a numeric output."
Copy link

Choose a reason for hiding this comment

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

I actually don't think this is overexplaining at all, especially if this would be some people's first time doing this sort of thing, which I can see being true.

Only note here: json -> JSON file

]
},
{
"cell_type": "code",
"execution_count": 2,
"metadata": {
"papermill": {
"parameters": {
"numeric_param": 1
}
},
"systemlink": {
"namespaces": [
"ni-testmanagement"
],
"outputs": [
{
"display_name": "Input plus fraction",
"id": "scalar_output",
"type": "scalar"
}
],
"parameters": [
{
"display_name": "Number",
"id": "numeric_param",
"type": "number"
}
],
"version": 2
},
"tags": [
"parameters"
]
},
"outputs": [],
"source": [
"# Parameters\n",
"numeric_param = 1 # default value used when input is not provided by Dashboard\n",
"\n",
"result = [] # container for all of the notebooks results"
Copy link
Contributor

Choose a reason for hiding this comment

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

Papermill works by injecting a new cell containing parameter value overrides directly below the cell marked parameters. Therefore, it's a good practice to keep any code that's not defining a default value for a parameter outside of the parameters cell. You're technically safe in this particular instance, but if this line depended on the parameter above, you can end up with some really weird (and hard to debug) behavior since that code would be referencing the default value, not the passed-in value.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also this line feels like it belongs in the "Results" cell a few cells down.

]
},
{
"cell_type": "code",
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks weird to have two code cells in a row without any description. Should we add markdown above this describing what this cell is doing? It would let the reader differentiate what's a parameter vs. what's implementation of their custom logic, in case they're using this as a template.

"execution_count": 3,
"metadata": {},
"outputs": [],
"source": [
"output = int(numeric_param) + uniform(0,1)"
Copy link
Author

Choose a reason for hiding this comment

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

This cast may not be needed anymore.

]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"## Results\n",
"\n",
"Scalar results are appended with a json map that corresponds to the json in the cell metadata described above. The \"type' and 'id' fields should match the cell metadata values, and the 'value' field should contain the scalar result. The config block doesn't correspond to anything in the cell metadata."
]
},
{
"cell_type": "code",
"execution_count": 4,
"metadata": {},
"outputs": [],
"source": [
"# Scalar result needs to be added to the result list and glued to the scrapbook\n",
"# The json descriptor should match the descriptor in the outputs section in the parameters cell metadata.\n",
"result.append({\n",
" 'type': 'scalar',\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Via a really roundabout way, the styleguide also recommends double-quotes for Python strings. Technically it just recommends any style rule that black also specifies: https://ni.github.io/python-styleguide/#F-1-1 and black specifies double quotes. We should probably start using double quotes for new examples - I'm aware that some older product code does not yet follow the newer conventions.

This is another example of the styleguide not perfectly applying to notebooks, as black doesn't work on notebooks natively. However, there are ways of getting this to be a thing, ping me if you're particularly curious about python linting.

" 'id': 'scalar_output',\n",
" 'config': {\n",
" 'title': 'Scalar Output Title'\n",
" },\n",
" 'value': output\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
" 'value': output\n",
" 'value': output,\n",

black also recommends trailing commas in multi-line lists and dictionaries. There are a few other places where this applies as well.

"})"
]
},
{
"cell_type": "code",
"execution_count": 5,
"metadata": {},
"outputs": [
{
"data": {
"application/scrapbook.scrap.json+json": {
"data": [
{
"config": {
"title": "Scalar Output Title"
},
"id": "scalar_output",
"type": "scalar",
"value": 1.322663202537357
}
],
"encoder": "json",
"name": "result",
"version": 1
}
},
"metadata": {
"scrapbook": {
"data": true,
"display": false,
"name": "result"
}
},
"output_type": "display_data"
},
{
"data": {
"text/plain": [
"[{'type': 'scalar',\n",
" 'id': 'scalar_output',\n",
" 'config': {'title': 'Scalar Output Title'},\n",
" 'value': 1.322663202537357}]"
]
},
"execution_count": 5,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"# Gluing the result list to the scrapbook is what actually allows you to bind the result to the numeric Dashboard tile.\n",
Copy link

Choose a reason for hiding this comment

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

I am loving this scrapbooking/gluing language. I didn't realize this was a thing, and I? Am delighted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note, you don't really have to do anything for this: Technically the styleguide recommends using 100 character lines, but that rule is particularly suited towards Python source files and makes less sense in notebooks. I don't think the conventions were really ever fully designed for notebooks because other rules don't apply whatsoever.
https://ni.github.io/python-styleguide/#F-1-2

Line length is fragmented in python and others agree on 120 (and this line is 119), so it might be fine to deviate here and keep it, since there's no reason to have such a short line length limit in a notebook outside of a .py file. Just something to be aware of.

"sb.glue('result', result)\n",
"result # this line is just here so you can see the result in Jupyter Hub and doesn't impact Dashboard."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"result # this line is just here so you can see the result in Jupyter Hub and doesn't impact Dashboard."
"display(result)"

Jupyter injects the display() builtin which is automatically called on the last expression of every cell. If we want to be explicit about this, we could just directly call it ourselves rather than needing a comment to explain it.

]
}
],
"metadata": {
"kernelspec": {
"display_name": "Python 3",
"language": "python",
"name": "python3"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.8.5"
}
},
"nbformat": 4,
"nbformat_minor": 4
}