Skip to content

Conversation

@sgoggins
Copy link

I'm going through the notebooks to make sure they run. Most of what is needed so far is :

  1. Creating some adaptability for the discovery of the config files, as notebooks are using differently named and configured configs.
  2. Ensuring credentials are pushed by updating the .gitignore
  3. Updating SQLAlchemy to reflect changes in how that library works with engine objects

This PR reflects updates to all notebooks at the root of notebooks/8knot

…g 3.12 and 3.13. Also added .venv folders to .gitignore

Signed-off-by: Sean P. Goggins <s@goggins.com>
Signed-off-by: Sean P. Goggins <s@goggins.com>
 Your branch is up to date with 'origin/spg-req-update-12-13'.

 Changes to be committed:
	modified:   notebooks/8knot/PR_response.ipynb
	modified:   notebooks/8knot/file_heatmap.ipynb
	new file:   notebooks/8knot/heatmap_directory.png
	modified:   notebooks/8knot/issue_first_response.ipynb
Signed-off-by: Sean P. Goggins <s@goggins.com>
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@sgoggins sgoggins added documentation Improvements or additions to documentation enhancement New feature or request dependencies Pull requests that update a dependency file labels Oct 22, 2025
@sgoggins sgoggins requested a review from cdolfi October 22, 2025 21:06
…per_60/github/oss-aspen/rappel/notebooks/8knot/non_viz/repo_information.ipynb NotJSONError('Notebook does not appear to be JSON: \'{\n cells: [\n {\n cell_type: m...)

"
;

Signed-off-by: Sean P. Goggins <s@goggins.com>
…ead.

Signed-off-by: Sean P. Goggins <s@goggins.com>
… import plotly-express and only plotly is installed

Signed-off-by: Sean P. Goggins <s@goggins.com>
Signed-off-by: Sean P. Goggins <s@goggins.com>
Signed-off-by: Sean P. Goggins <s@goggins.com>
@MoralCode
Copy link

holy diff batman

@cdolfi
Copy link
Contributor

cdolfi commented Nov 14, 2025

@MoralCode yeahhh lol I dont even know how to get started on this one

@cdolfi
Copy link
Contributor

cdolfi commented Nov 24, 2025

@sgoggins Can you drop all the files from this PR that are just a cell output/print change?

@MoralCode
Copy link

MoralCode commented Nov 25, 2025

after a quick re-review of the diff, it looks like this PR is removing a lot of the output values from the notebooks. although yeah that should probably be a separate PR.

unsure if theres a script that can clear this output for all notebooks or if you need to go one by one, but I strongly suspect that doing this output removal as a new PR, merging it, and then rebasing this PR/branch will help reveal the substantial, non-output changes that are made here (or reveal places where this PR is adding output). Maybe thats something you can do @cdolfi ? (or I can do this next week)

This may also help clean things up prior to the chaoss transfer too. (and could maybe be enforced with a GH action that flags PRs that contain non-cleared outputs so that contributors are consistently required to keep them clear)

@cdolfi
Copy link
Contributor

cdolfi commented Nov 25, 2025

@MoralCode I actually do not want to remove the outputs as they are useful to seeing what data the notebooks are working with. This is pretty common practice when distributing research notebooks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants