search

jerry-git / learn-python3

Jupyter notebooks for teaching/learning Python 3
MIT License
6.43k stars 1.77k forks source link

Undefined name 'c' in post_save_hook.py #1

Closed cclauss closed 6 years ago

cclauss commented 6 years ago

flake8 testing of https://github.com/jerry-git/learn-python3 on Python 3.6.3

$ flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics

./utils/post_save_hook.py:98:1: F821 undefined name 'c'
c.FileContentsManager.post_save_hook = post_save^
1     F821 undefined name 'c'
1
jerry-git commented 6 years ago

Thanks for the report!

First of all, I have to say that I'm a big fan of flake8 and linting in general.

The file that is triggering the error is a post save hook implementation that should be added as part of Jupyter configuration file (jupyter_notebook_config.py). The c refers to Config object which is automatically created and will be in the namespace when the post save hook implementation is loaded, see https://traitlets.readthedocs.io/en/latest/config.html#python-configuration-files.

I think the only reasonable way for fixing the linting error would be to add # noqa for that line. However, the post save hook implementation file is actually the only .py file currently in the project. Thus, I think setting up linting, especially for CI, is a bit overkill here.

I may use some .py files as complementary materials in the future but I'm not sure if linting will make sense for those either. For example, if exercises would contain .py files, those files would most likely include some errors / missing code that the learner should fix. Linter would not be happy about this.

However, it would maybe make sense to have some sort of linting for the notebooks, excluding the exercise notebooks. If someone knows a solution which does not introduce any boilerplate to notebook cells, I'd be interested. I checked the different approaches presented here https://stackoverflow.com/questions/26126853/verifying-pep8-in-ipython-notebook-code but I don't think any of those is suitable for my use case.

cclauss commented 6 years ago

Agreed. Closing. For reference, the other way to placate flake8 on this score is to declare global c.