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

Fixing errors in CI #94

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fixing errors in CI #94

wants to merge 3 commits into from

Conversation

snbianco
Copy link
Contributor

@snbianco snbianco commented Feb 4, 2025

Updating the notebooks that are failing in our nightly CI build.

@snbianco snbianco requested a review from ttdu as a code owner February 4, 2025 16:45
@snbianco
Copy link
Contributor Author

snbianco commented Feb 4, 2025

Tagging @adrianlucy @scfleming for visibility

@snbianco snbianco requested a review from adrianlucy February 4, 2025 17:40
@adrianlucy
Copy link
Contributor

adrianlucy commented Feb 4, 2025

Thanks Sam! Will review ~Thursday-ish, but one quick minor thing that caught my eye: diff changes from "execution_count": null to e.g. "execution_count": 1 suggest some cells may be executed in this commit? I think preference is for notebooks to be unexecuted in this repo, mostly (I think?) to save on space, also makes git diffs easier to read, maybe other reasons that no one ever told me.

In theory if the unwanted output in this PR takes up significant storage, I guess you might want to consider squashing the commits that take up space in the history?, but up to you / judgement call whether that's worth your time.

@snbianco
Copy link
Contributor Author

snbianco commented Feb 4, 2025

Thanks Sam! Will review ~Thursday-ish, but one quick minor thing that caught my eye: diff changes from "execution_count": null to e.g. "execution_count": 1 suggest some cells may be executed in this commit? I think preference is for notebooks to be unexecuted in this repo, mostly (I think?) to save on space, also makes git diffs easier to read, maybe other reasons that no one ever told me.

In theory if the unwanted output in this PR takes up significant storage, I guess you might want to consider squashing the commits that take up space in the history?, but up to you / judgement call whether that's worth your time.

That is weird, apparently the "Clear All Outputs" button in VSCode doesn't actually reset the execution count! The notebooks shouldn't actually have any output. I'll see if I can find a way to reset that variable.

@adrianlucy
Copy link
Contributor

adrianlucy commented Feb 4, 2025

Oh interesting, weird! At least we don't have to worry about the commit history then. FWIW in Jupyter itself I think it's "restart kernel and clear outputs"(?) but I can see it being a hassle to switch between VScode's implementation and Jupyter.

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 this pull request may close these issues.

2 participants