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

jupytext is being run on files that are not generated by jupytext #753

Closed
francesco-ballarin opened this issue Sep 28, 2022 · 8 comments · Fixed by #754
Closed

jupytext is being run on files that are not generated by jupytext #753

francesco-ballarin opened this issue Sep 28, 2022 · 8 comments · Fixed by #754

Comments

@francesco-ballarin
Copy link

Hi,
my understanding is that #745 introduced support for jupytext, and that
b95808d#diff-d72c6dc865400b582f957459d2f13e4030da8a9f8afdec522def74313d29a25fR129
should be discarding any markdown files not generated by jupytext.

Is that correct?

If so, what I observe in one of my repositories is that README.md, which clearly was not generated by jupytext, is still getting processed by nbqa flake8. This in turn might be causing unexpected CI failures.

@MarcoGorelli
Copy link
Collaborator

thanks @francesco-ballarin - yes, that's indeed correct

could you link me to your README please? I'd tried this out on a few readmes and they were indeed being ignored

@francesco-ballarin
Copy link
Author

A README of one of the projects I see CI failure is
https://github.com/multiphenics/multiphenicsx/blob/main/README.md

Note that I would not get nbqa flake8 failures if I did not have the additional flake8-docstrings plugin enabled in
https://github.com/multiphenics/multiphenicsx/blob/main/setup.cfg#L56

It makes no sense for that plugin to throw a warning in the case of jupyter notebooks, and indeed at
https://github.com/multiphenics/multiphenicsx/blob/main/setup.cfg#L90
I am ignoring D100 error messages of python files derived from ipynb notebooks.

I could happily live with ignoring D100 errors on README.md as well, but the point of this issue is that I shouldn't have to, since README.md definitely does not come from jupytext.

The relevant call on the CI workflow is at
https://github.com/multiphenics/multiphenicsx/blob/main/.github/workflows/ci.yml#L72

@MarcoGorelli
Copy link
Collaborator

I could happily live with ignoring D100 errors on README.md as well, but the point of this issue is that I shouldn't have to, since README.md definitely does not come from jupytext.

Yes, agreed - thanks for the report, I'll take a look!

@francesco-ballarin
Copy link
Author

Yes, agreed - thanks for the report, I'll take a look!

Thanks! :)

@MarcoGorelli
Copy link
Collaborator

@francesco-ballarin this should be fixed in version 1.5.2, could you please check?

@francesco-ballarin
Copy link
Author

Thanks for the quick fix.

I need to wait for FEniCS/dolfinx#2384 before I can check the fix in v1.5.2 (your fix and that PR are totally unrelated, but my current main branch was affected by both). I'll report back here as soon as I can test v1.5.2.

@francesco-ballarin
Copy link
Author

Hi @MarcoGorelli, I have tested v1.5.2 on my CI and I can indeed confirm that it fixes the issue on README.md. Thanks again for the quick fix.

@MarcoGorelli
Copy link
Collaborator

nice, thanks for checking (and for the report )!

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