Jupyter notebook support

[chadsr]

Since psf/black#2357 was merged, black supports linting Jupyter notebook files (.ipynb).
This PR therefore aims to extend the action to also annotate Jupyter notebook files.

I'm not sure how this will play with Github's displaying of Jupyter notebook code, or whether the output format from black for Jupyter is different from normal python file output, so this may need more testing/adaptation.

P.S. I debated adding an extra with arg for this, but then figured it's easier to just use the existing black_args , if you wish to exclude/disable .ipynb checks, but maybe a README remark on this is useful?

Input/opinions/edits welcome.

[chadsr]

I couldn't manage to test this properly on my local machine due to #45, so please feel free to ping me if there are test/build issues that still need resolving.

[rickstaa]

@chadsr Thanks a lot for implementing this pull request! I noticed you transferred your pull request to a draft. Feel free to let me know when you are ready. I added one small commit to account for the false positives received from the spellcheck linter.

[chadsr]

@rickstaa I've added some Bash magic, so that the test actually checks the output of black is as expected.

Beyond that, i'm not yet totally sure how to verify if reviewdog handles the .ipynb annotations "properly" in Github?

Edit: whoops, now I realise that the new test only covers pushes, not PRs. Let me fix that up.
Edit 2: Actually, since test-check always runs, even during the pull_request event, I think this might be sufficient as is?

[chadsr]

Either I am missing something, or the .ipynb file annotation does not work properly, yet (probably due to structural differences in the file vs black output).

I would expect to see the reviewdog annotations added to this pull request, correct?

Edit: Bummer, it looks like Black does some diff formatting to make it more human readable, with the caveat that the diff line numbers outputted are now relative to each cell in the notebook:

--- testdata/num_guess.ipynb	2021-12-09 22:07:38.014000 +0000:cell_0
+++ testdata/num_guess.ipynb	2021-12-10 10:08:23.231019 +0000:cell_0
@@ -1,10 +1,10 @@
 """Small test script taken from https://wiki.python.org/moin/SimplePrograms"""

 import random
-import sys # F401 'os' imported but unused
-import os # F401 'os' imported but unused
+import sys  # F401 'os' imported but unused
+import os  # F401 'os' imported but unused

 ### E265 block comment should start with '# '
 print("Hello from reviewdog!")
 print("Let's play a small number guessing game to test the flake8 github action.")
 print("This game is taken from https://wiki.python.org/moin/SimplePrograms.")
--- testdata/num_guess.ipynb	2021-12-09 22:07:38.014000 +0000:cell_1
+++ testdata/num_guess.ipynb	2021-12-10 10:08:23.231019 +0000:cell_1
@@ -1,11 +1,13 @@
 guesses_made = 0

 name = input("Hello! What is your name?\n")

 number = random.randint(1, 20)
-print("Well, {0}, I am thinking of a number between 1 and 20.".format(name)) # E501 line too long (80 > 79 characters)
+print(
+    "Well, {0}, I am thinking of a number between 1 and 20.".format(name)
+)  # E501 line too long (80 > 79 characters)

 while guesses_made < 6:

     guess = int(input("Take a guess: "))

@@ -27,6 +29,6 @@
         )
     )
 else:
     print("Nope. The number I was thinking of was {0}".format(number))

-import itertools # E402 module level import not at top of file
+import itertools  # E402 module level import not at top of file
would reformat testdata/num_guess.ipynb

So I think to make annotations work properly for this PR, it's going to require adaptation to the reviewdog diff parser, or some hacky way in this repo (or thirdly, Black to implement a "raw diff" option)

[chadsr]

Should we maybe come to a decision, as to whether to put this PR on hold until annotations are definitely working properly, or to merge this and open an issue/PR to figure out the absolute vs relative line number problem?

Having some form of output as seen in https://github.com/reviewdog/action-black/actions/runs/1562786562 still seems like a good step in the right direction, and possibly worth of merging as-is, for now.

Ah, and this issue answers my confusion around the annotations not showing up for this PR (it's because it's from my fork): reviewdog/action-tflint#2

[rickstaa]

@chadsr Thanks again for your pull request. I haven't yet been able to find the time to review your pull request properly. I try to do it this or next weekend. You can also try requesting a review from the other maintainers to speed the process along.

[chadsr]

@rickstaa Whoops, this PR slipped my mind. No problem.

Technically I can merge this myself, now that I joined the reviewdog org, but it would still be good to get a second opinion, I think?

[rickstaa]

@chadsr I understand! I will try to squeeze in some time to review it this weekend.

[chadsr]

On the subject of dedicating time to this repo, I should also add, that moving forward I will happily take on a pro-active maintainer role to spread the load.

• and on this subject, maybe it's useful for us to add a .github/CODEOWNERS file, so that contributors can more easily know who to ping, or even better, just let Github automatically assign reviewers to PRs.

[rickstaa]

On the subject of dedicating time to this repo, I should also add, that moving forward I will happily take on a pro-active maintainer role to spread the load.

• and on this subject, maybe it's useful for us to add a .github/CODEOWNERS file so that contributors can more easily know who to ping, or even better, just let Github automatically assign reviewers to PRs.

That's very nice! 🚀 I also like your ideas. I, unfortunately, still haven't found the time to review the pull request but I will look at it this weekend.

[rickstaa]

@rickstaa I've added some Bash magic, so that the test actually checks the output of black is as expected.

Beyond that, i'm not yet totally sure how to verify if reviewdog handles the .ipynb annotations "properly" in Github?

Edit: whoops, now I realise that the new test only covers pushes, not PRs. Let me fix that up. Edit 2: Actually, since test-check always runs, even during the pull_request event, I think this might be sufficient as is?

@chadsr You are right the new test should be enough I like it! I extended it a bit to test both files that should be checked and the ones that should not be checked (see
3dc3b32).

[rickstaa]

@chadsr I reviewed your pull request this morning, the code looks very clean and it works (see https://github.com/rickstaa/action-test-repo/actions/runs/1822872981). I agree that we might be able to improve the review dog output later but I think for now your pull request already is very valuable. Thanks again for implementing it! 🚀

[rickstaa]

@chadsr Feel free to merge it after you reviewed 89696f9.

[github-actions]

🚀 [bumpr] Bumped! New version:v3.1.0 Changes:v3.0.1...v3.1.0

[rickstaa]

@chadsr Thanks for merging this pull request.

Please be aware that with the current release action https://github.com/reviewdog/action-black/actions/workflows/release.yml the GitHub marketplace version does not automatically update. As a result, after each release, we have to manually click the edit button and save it for the version to update in the marketplace.

I still have to change this to using the gh https://github.com/cli/cli (see reviewdog/reviewdog#1043), but I haven't found the time yet. I will create a pull request to remind us to change this.

[github-actions]

🚀 [bumpr] Bumped! New version:v3.2.0 Changes:v3.1.0...v3.2.0

[github-actions]

🚀 [bumpr] Bumped! New version:v3.3.0 Changes:v3.2.0...v3.3.0