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

Not compatible with pre-commit-hooks/pretty-format-json hook #185

Open
rusmux opened this issue Sep 7, 2023 · 6 comments
Open

Not compatible with pre-commit-hooks/pretty-format-json hook #185

rusmux opened this issue Sep 7, 2023 · 6 comments
Labels
state:needs follow up This issue has interesting suggestions / ideas worth following up on type:dependency

Comments

@rusmux
Copy link

rusmux commented Sep 7, 2023

When using this tool with pretty-format-json hook from https://pre-commit.com/hooks.html, when they both rerun after each other regardless of the order. I suppose one way to fix this is to make nbstripout compatible with pretty-format-json, i.e. it won't run again after pretty-format-json formatting

@rusmux rusmux changed the title Not compatible with pre-commit/pretty-format-json hook Not compatible with pre-commit-hooks/pretty-format-json hook Sep 7, 2023
@dokempf
Copy link

dokempf commented Oct 4, 2023

Hey @rusmux, I was running into the same situation and took a deep dive at what is happening. Here are the results

The root of the issue is a change in the identify library that provides pre-commit with information about what files to apply what hooks to. Recently somebody added the information that an ipynb file is also a json file: pre-commit/identify@cfd2188 If the identify version on your system (which is versioned completely independently from pre-commit and thus we have no control over from a hook) happens to include that commit, the pretty-format-json hook will run on notebooks although it previously did not. The result is that nbstripout and pretty-format-json step on each others toes.

The "proper" way to fix this would be to align the output of the two tools (they differ in indentation mostly), but I had a hard time reading nbstripout code to find the places where the actual formatting is happening. There is a stop gap measure though, which is to disable the pretty-format-json hook for jupyter files in your configuration like this:

repos:
  - repo: https://github.com/kynan/nbstripout
    rev: 0.6.1
    hooks:
      - id: nbstripout
        files: ".ipynb"

  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.4.0
    hooks:
      - id: pretty-format-json
        exclude_types:
          - jupyter
        args:
          - --autofix

@rusmux
Copy link
Author

rusmux commented Oct 4, 2023

@dokempf Great job! I will then follow your example, thank you!

@kynan
Copy link
Owner

kynan commented Oct 21, 2023

@dokempf Thanks for digging into this! The reason you didn't find the code where nbstripout does the formatting is that it doesn't 😃 It uses nbformat, so if you want to lobby for a change, you need to file a bug against nbformat.

@kynan kynan added state:needs follow up This issue has interesting suggestions / ideas worth following up on type:dependency labels Oct 21, 2023
@dokempf
Copy link

dokempf commented Oct 27, 2023

It seems like nbformat has discarded the idea a while back: jupyter/nbformat#228

@kynan
Copy link
Owner

kynan commented Nov 16, 2024

Is this still a pain point for you @rusmux @dokempf ?

@rusmux
Copy link
Author

rusmux commented Nov 17, 2024

No, I'm using @dokempf suggested solution:

- id: pretty-format-json
        exclude_types:
          - jupyter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:needs follow up This issue has interesting suggestions / ideas worth following up on type:dependency
Projects
None yet
Development

No branches or pull requests

3 participants