Skip to content

spellcheck#1413

Closed
ocefpaf wants to merge 9 commits intopython-visualization:mainfrom
ocefpaf:spellcheck
Closed

spellcheck#1413
ocefpaf wants to merge 9 commits intopython-visualization:mainfrom
ocefpaf:spellcheck

Conversation

@ocefpaf
Copy link
Copy Markdown
Member

@ocefpaf ocefpaf commented Nov 9, 2020

The failures here are related to things that will be removed in #1410, so no need to fix them.

Comment thread tests/test_jinja.py Outdated
@ocefpaf
Copy link
Copy Markdown
Member Author

ocefpaf commented Nov 29, 2020

@Conengmo the spellcheck won't pass b/c there are false positives. Codespell is inspecting the notebook outputs and they contain all sort of strings in the embed images and other outputs.

When we start building a gallery from "clean" notebooks this will pass. Still, I believe that there is value in merging now so we can check new code for spell errors.

@ocefpaf
Copy link
Copy Markdown
Member Author

ocefpaf commented Dec 29, 2021

@Conengmo can you take a second look at this one? I removed all the outputs from all the notebooks. They should never be there in the first place. I removed the GHA check for the pre-commit and replaced it with pre-commit CI instead. Faster and it will update the pre-commit-ci file for us.

I believe that the examples should be run on binder or locally, so removing the outputs not only make the diffs more meaningful and the repo easier to manage but also "force" folks to run them first before copying and pasting from them.

@Conengmo
Copy link
Copy Markdown
Member

Conengmo commented Jan 4, 2022

I removed all the outputs from all the notebooks

Doesn't that mean the nbviewer links will be disabled as well? I don't think that's acceptable, as examples they should be easy to view, without having to wait to spin up a binder.

so removing the outputs not only make the diffs more meaningful and the repo easier to manage

that's true, Notebook diffs are terrible. We could also solve that by having a 'source' folder without rendering, and a 'main' folder that contains copies that get rendered on releases.

Or, I'd rather have to hard-to-use diffs then break our nbviewer examples.

but also "force" folks to run them first before copying and pasting from them.

I don't think that's a benefit.

@ocefpaf
Copy link
Copy Markdown
Member Author

ocefpaf commented Jan 4, 2022

We could also solve that by having a 'source' folder without rendering, and a 'main' folder that contains copies that get rendered on releases.

That is probably be best option. A while back I wanted to move all examples to another repo so we are not held accountable for maintaining them but I guess that ship has sailed.

Let's move this one to a draft again and I'll try to work on that solution.

@ocefpaf ocefpaf marked this pull request as draft January 4, 2022 18:07
@Conengmo
Copy link
Copy Markdown
Member

Superseded by #1652

@Conengmo Conengmo closed this Nov 17, 2022
@ocefpaf ocefpaf deleted the spellcheck branch November 17, 2022 14:44
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