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

Upgrade from traitlets 5.11.2 to 5.12.0 broke script #887

Closed
wesk opened this issue Oct 30, 2023 · 1 comment · Fixed by #903
Closed

Upgrade from traitlets 5.11.2 to 5.12.0 broke script #887

wesk opened this issue Oct 30, 2023 · 1 comment · Fixed by #903

Comments

@wesk
Copy link

wesk commented Oct 30, 2023

Context

I noticed that a script that we use as part of our CI workflow stopped working with the update from traitlets 5.11.2 to 5.12.0.

The script in question: https://github.com/awslabs/syne-tune/blob/main/.github/workflows/utils/notebooks2scripts.py#L8

Relevant lines:

from traitlets.config import Config
from nbconvert.exporters import PythonExporter

c = Config()
c.TagRemovePreprocessor.remove_cell_tags = "ci-skip-cell"
python_exporter = PythonExporter(config=c)

The script converts Jupyter notebooks into Python scripts that our CI can execute, to make sure that our notebooks work correctly. For more about our use-case, refer to the PR that added it: syne-tune/syne-tune#756

We have been using traitlets to skip specific Jupyter notebook cells that have been tagged with a special ci-skip-cell tag (example). The reason for skipping specific cells is because we found that attempting to convert notebooks with %pip install cells did not work. What we are doing is instead installing the dependencies ahead of time in our CI environment, and then running the rest of the converted script.

Screenshot 2023-10-30 at 11 20 53 PM

Problem

Before the update to 5.12.0, running the conversion script would skip the cells tagged with ci-skip-cell, however after the update, the cells have stopped being skipped.

Converted script before updating:

#!/usr/bin/env python
# coding: utf-8

# # Tune XGBoost

# ### Install dependencies
# 

# In[ ]:

... (truncated)

Converted notebook after the update:

#!/usr/bin/env python
# coding: utf-8

# # Tune XGBoost

# ### Install dependencies
# 

# In[ ]:


get_ipython().run_line_magic('pip', "install 'syne-tune[basic]'")
get_ipython().run_line_magic('pip', 'install xgboost')


# In[ ]:


... (truncated)

Note the two additional get_ipython lines that we do not want to see.

PR reverting to 5.11.2 makes all of the test pass again: syne-tune/syne-tune#782

Example test run where they were failing: https://github.com/awslabs/syne-tune/actions/runs/6693658169

Question

Is the new traitlets behavior expected?

@azjps
Copy link
Collaborator

azjps commented Apr 18, 2024

This looks like a similar issue caused by the typo described in #891.

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 a pull request may close this issue.

2 participants