Before refactoring a notebook, the user must ensure that the original notebook runs. We should have a command to check whether it works and suggest actions if if there are some errors.

e.g.,

soorgeon run nb.ipynb

If the notebook fails because of a ModuleNotFound: suggest creating a virtualenv, and adding a requirements.txt with the package name

If it's an other error, show the guide for debugging notebooks

if it's a function signature, recommend downgrading some libraries

An alternative approach would be to let the user convert anyway and then help them fix the issues in the ploomber generated pipeline, this way they can leverage incremental builds for rapid iterations. we can also suggest them to add a sample parameter

(i think this is already covered but I need to make sure and add a few tests)

Important: for these examples comments are H2 headings (e.g. # I'm an H2 heading)

We have to add two test cases:

1. Overriding variables

# first

df = something()

# second

df = another()

# third

df_2 = df + 1

When running soorgeon refactor, the output should contain three notebooks (first, second, third). third.ipynb must contain the following:

# third

df_2 = df + 1

Now, third uses df as input. So we must ensure that soorgeon picks up the df from the second notebook and not the one from the first one. Graphically it should be:

  graph LR;
      first;
      second --> third;

Overriding variables (same cell)

# first
df = something()

# second
df = another()
df_2 = df + 1

We need to ensure that in this case, the second task generated does not create a dependency with the first task, since it has its own df (i.e., it does not depend on the df = something() line).

In other words, the output pipeline (produced by ploomber plot), should look like this (two independent tasks):

  graph LR;
      first;
      second;

Instead of this (two dependent tasks):

  graph LR;
      first --> second;

testing

the simplest way to test it is to call soorgeon refactor and then load the pipeline to ensure we got the right structure:

from ploomber.spec import DAGSpec

nb = """
# notebook content goes here
"""

# refactor notebook
export.from_nb(_read(nb))

dag = DAGSpec('pipeline.yaml').to_dag().render()

# write assert statements for each task in the pipeline
# to get the task names, you can do:
list(dag)

# example, check that task_name only has one upstream dependency called 'get'
assert  set(dag['task_name'].upstream) == {'get'}

# another example, check that another_task has no dependencies
assert  set(dag['another_task'].upstream) == set()

when running soorgeon clean we convert the notebook to .py and then back to .ipynb the ploomber is that cell outputs are lost. What we want instead is to keep the outputs as they were. I think there might be some challenges since running isort maybe cause the clean notebook to have extra cells (and potentially running black, too). so we need to find a solution. I think the new jupyter notebook format adds unique cell ids, so we can use that (if the notebook does not have the cell ids, maybe loading it with nbformat.load will add them). worst case: we create our cell ids

Some objects aren't pickable (e.g, jinja templates) so we should capture this errors at runtime and suggest fixes:

1. use another library like cloudpickle (this can be an option), like sorgeon notebook.ipynb --serializer cloudpickle
2. link to a guide to fix the problem (e.g., create a factory to instantiate the object)

It's possible that the original notebook already creates new files, we should show a warning if that's the case and tell the use to register them as products (include a short guide to explain how to do it)

We have a bunch of integration tests that fetch data and notebooks using Kaggle's API. However, they fail sometimes (probably because the Kaggle API isn't very reliable). See https://github.com/ploomber/soorgeon/issues/58

One solution is to use the Kaggle API once and then upload the files to an S3 bucket, then have the CI download them from S3 instead of Kaggle.

I ran soorgeon refactor on this notebook: https://www.kaggle.com/code/cdeotte/xgboost-starter-0-793

after fixing the headings and the function that uses global variables. I got this error:

  File "/Users/Edu/dev/soorgeon/src/soorgeon/io.py", line 515, in find_inputs_and_outputs_from_leaf
    (_, candidates_in, candidates_out) = find_for_loop_def_and_io(
  File "/Users/Edu/dev/soorgeon/src/soorgeon/io.py", line 104, in find_for_loop_def_and_io
    raise ValueError(f'Expected a node with type "for_stmt", '
ValueError: Expected a node with type "for_stmt", got: <ExprStmt: df = df.merge(importances[k], on='feature', how='left')@4,25> with type expr_stmt
Error: An error occurred when refactoring notebook.

and it failed. I realized it's because of this for loop:

for k in range(1,FOLDS): df = df.merge(importances[k], on='feature', how='left')

I realized that applying black and rerunning it fixed the issue. that's an easy fix for many of this edge cases; so I think we should apply black on the notebook before running soorgeon refactor

here's the notebook. note that I changed the extension from ipynb to txt since github wont let me upload it with ipynb extension

xgboost-starter-0-793.txt

Describe your changes

Parse notebook code, if it contains statement such as open, output warning.

Issue ticket number and link

Closes #16

Checklist before requesting a review

• [x] I have performed a self-review of my code
• [x] I have added thorough tests (when necessary).
• [x] I have added the right documentation (when needed). Product update? If yes, write one line about this update.

Describe your changes

1. Instead of fetching from kaggle, we fetch from github by pygithub api now
2. Add _pygithub.py for downloading a dir from a repo.
3. Add pygithub package to setup.py (I don't know if this one is necessary)
4. Removed stored notebooks under _kaggle since we can store them in the storage repo now
5. Removed unused functions under src/_kaggle.py
6. Removed kaggle username and key in CI, I don't think we need them. Maybe we can also remove them from repo secrets
7. Storage repo here and README.md has instructions on how to upload
8. Contribute.md remains unchanged, we can let user register new notebook in _kaggle/index.yaml. But the rest of downloading and uploading, updating CI and test cases will need to be done manually by one of the team members.

To do

1.We need to add a personal access token to the repo's secrets. 2. Right now CI will fail because this one fails test_notebooks[look-at-this-note-feature-engineering-is-easy] for this reason: E soorgeon.exceptions.InputError: Only H1 headings are found. At this time, only H2 headings are supported. Check out our guide:

Issue ticket number and link

Closes #59 #39

Checklist before requesting a review

• [x] I have performed a self-review of my code
• [x] I have added thorough tests (when necessary).
• [x] I have added the right documentation (when needed). Product update? If yes, write one line about this update.

The PR tries to solve https://github.com/ploomber/soorgeon/issues/9

1. Added a CLI option called serializer which can be one of cloudpickle or dill
2. Based on the serializer selected soorgeon will try to pickle the objects with either cloudpickle or dill
3. Both the packages are optional dependencies

Describe your changes

• Refactor test command to use papermill to run notebooks
• Add file existence check for output of executed notebook

Issue ticket number and link

Closes #6

Checklist before requesting a review

• [x] I have performed a self-review of my code
• [x] I have added thorough tests (when necessary).
• [x] I have added the right documentation (when needed). Product update? If yes, write one line about this update.

Previously, we did not support using global variables inside a function's body:

        x = 1
        def sum(y):
            return x + y
        sum(y)

The above would break since sum is using a variable that's defined outside the function's body. If a user tried to refactor a notebook with code like this using Soorgeon refactor, they would get an error message asking them to change the code to:

        x = 1
        def sum(y, x):
            return x + y
        sum(y, x)

This PR automate this process and modify the user's source code on their behalf so they don't have to do it manually.

https://github.com/ploomber/soorgeon/issues/65

Closes #65

Checklist before requesting a review

• [ x] I have performed a self-review of my code
• [ x] I have added thorough tests (when necessary).
• [ ] I have added the right documentation (when needed). Product update? If yes, write one line about this update.

saw this on stack overflow: https://stackoverflow.com/questions/41046955/formatting-sql-query-inside-an-ipython-jupyter-notebook

it could work both on cells that have:

%%sql

and markdown snippets

Currently, we do not support using global variables inside a function's body:

x = 1

def sum(y):
  return x + y

sum(y)

The above will break since sum is using a variable that's defined outside the function's body. If a user tries to refactor a notebook with code like this using soorgeon refactor, they'll get an error message asking them to change the code to:

x = 1

def sum(y, x):
  return x + y

sum(y, x)

We throw an error message a link to this document.

However, we should automate this process and modify the user's source code on their behalf so they don't have to do it manually.

considerations

There are a few edge cases to take into account, for example, what if the function already has an argument with the name? Or what if the function's signature is using *args, or **kwargs. Since there are many edge cases, we should focus on covering the simple ones to ensure it works, detect a few of the edge ones, and throw an error so the user fixes it manually.

modifying users' code

soorgeon refactor parses the code into the AST to detect dependencies among notebook's. Python's standard library has an ast module; however, it's very limited so we use parso instead.

Parso offers roundtrip conversion, meaning we can go from source code to AST and to source code again. You can see an example of that here:

https://github.com/ploomber/soorgeon/blob/03229806b905e7715d1c3ee0413ff5a3bc30b71c/src/soorgeon/io.py#L851

The above is a function that removes import statements from a string with source code.

determining if parso is the best option

so far, parso has worked well for us; however, we are interested in exploring other options. We're interested in improving soorgeon's capabilities to automatically refactor code so we should ensure we're using the right tool for the job. so part of fixing this issue is to see if there are better alternatives.

the bottom of Python's AST module links to some alternatives:

See also

Green Tree Snakes, an external documentation resource, has good details on working with Python ASTs.

ASTTokens annotates Python ASTs with the positions of tokens and text in the source code that generated them. This is helpful for tools that make source code transformations.

leoAst.py unifies the token-based and parse-tree-based views of python programs by inserting two-way links between tokens and ast nodes.

LibCST parses code as a Concrete Syntax Tree that looks like an ast tree and keeps all formatting details. It’s useful for building automated refactoring (codemod) applications and linters.

Parso is a Python parser that supports error recovery and round-trip parsing for different Python versions (in multiple Python versions). Parso is also able to list multiple syntax errors in your python file.

And I also found this other project:

https://github.com/PyCQA/redbaron https://github.com/PyCQA/baron

we started working on a soorgeon clean command to apply flake8 and isort to notebooks (see https://github.com/ploomber/soorgeon/issues/50)

We can extend this functionality to do other types of clean-ups. PyCQA hosts a bunch of interesting projects. a few ones that caught my eye:

autoflake: https://github.com/PyCQA/autoflake bandit: https://github.com/PyCQA/bandit

a similar package for inspiration: https://github.com/nbQA-dev/nbQA

Notebooks get messy. Suppose a data scientist is working on a notebook that downloads some data, cleans it, generates features, and trains a model. Let's now say we want to deploy this notebook so we run a scheduled job every month to re-train the model with new data. In production, we need to keep some of the notebook logic (but not all).

During development, data scientists usually add extra cells for exploration and debugging purposes. For example, I might add a histogram to see the distribution of the input features to see if the data exhibits certain properties. While useful during development, some of the notebook's cells (e.g. plotting a histogram) aren't needed for production, so cleaning the notebook will be useful for a smoother transition to production.

Note that this is related to soorgeon clean (https://github.com/ploomber/soorgeon/issues/50) but not the same. soorgeon clean only reformats the existing code. In this case, we're talking about making deeper changes to the user's code.

low-hanging fruit: using autoflake

The low-hanging fruit here is to remove unused imports and variables via autoflake. For example, say a notebook looks like this:

# cell 1
import math
import pandas as pd
df = pd.read_csv('data.csv')

# cell 2
df2 = pd.read_csv('another.csv')

# cell 3
df.plot()

if we run autoflake on that notebook, it'll be able to delete import math since the module is never used, and also df2 = pd.read_csv('another.csv') since df2 isn't used, and we'll end up with:

# cell 1
import pandas as pd
df = pd.read_csv('data.csv')

# cell 2
df.plot()

more advanced approach: automated variable pruning

A more advanced approach would be to delete everything that is not required to produce some final result. For example, say we have a notebook whose final result is to produce df_final:

import pandas as pd
df = pd.read_csv('data.csv')
df2 = pd.read_csv('another.csv')
df3 = pd.read_csv('final.csv')

df4 = do_something(df2, df3)

df_final = do_stuff(df1)

We can work backward and eliminate everything that does not affect df_final:

import pandas as pd
df = pd.read_csv('data.csv')

df_final = do_stuff(df1)

considerations

Pruning code is useful for cleaning up that's not needed for deployment but in some cases; the code might be needed again. For example, if I add some data exploration code (i.e. code to generate a plot), I may want to delete it as part of this automated cleaning process, but once the model is deployed, I might need that code again if the model fails and I need to debug things; I'm unsure on how to deal with this scenario