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

Multiline terminal command isn't processed properly #657

Closed
ivanmkc opened this issue Sep 1, 2021 · 12 comments
Closed

Multiline terminal command isn't processed properly #657

ivanmkc opened this issue Sep 1, 2021 · 12 comments

Comments

@ivanmkc
Copy link

ivanmkc commented Sep 1, 2021

I've included two notebooks, which are identical except for the terminal command.

  • one with a single-line terminal command
! bq --location=US load --autodetect --skip_leading_rows=1 --source_format=CSV {dataset_id}.us_states_local_file 'resources/us-states.csv'
  • one with a multi-line terminal command
!bq \
--location=US load \
--autodetect \
--skip_leading_rows=1 \
--source_format=CSV \
{dataset_id}.us_states_local_file \
'resources/us-states.csv'

The one with multiline doesn't process properly, so when I run nbqa with black, I get:

Running black...
error: cannot format notebooks/official/bigquery_command_line_tool.ipynb: Cannot parse: cell_6:2:5: --lo 0x6AA55214

Archive.zip

@ivanmkc ivanmkc changed the title Multiline magic isn't processed properly Multiline terminal command isn't processed properly Sep 1, 2021
@loferris
Copy link

loferris commented Sep 1, 2021

I have had an almost identical error with this file and in addition
!bq load \ --location=US \
etc. is auto-revised by black to the second block @ivanmkc mentions above, which then throws the same error. This syntax should also be acceptable. https://cloud.google.com/bigquery/docs/bq-command-line-tool#loading_data

@MarcoGorelli
Copy link
Collaborator

MarcoGorelli commented Sep 2, 2021

Hey @loferris @ivanmkc , thanks for the report! I'll take a look

Does this also happen when you run black on the notebook directly? (As of version 21.8b0, it has support for notebooks)

@MarcoGorelli
Copy link
Collaborator

Can't reproduce either example with the latest version (1.1.0):

(venv) marcogorelli@OVMG025 mcve % cat Untitled.ipynb 
{
 "cells": [
  {
   "cell_type": "code",
   "execution_count": null,
   "id": "f013878e-4ee4-4d12-ba40-2afac49d4c29",
   "metadata": {},
   "outputs": [],
   "source": [
    "!bq \\\n",
    "--location=US load \\\n",
    "--autodetect \\\n",
    "--skip_leading_rows=1 \\\n",
    "--source_format=CSV \\\n",
    "{dataset_id}.us_states_local_file \\\n",
    "'resources/us-states.csv'\n"
   ]
  }
 ],
 "metadata": {
  "kernelspec": {
   "display_name": "good-bayesian",
   "language": "python",
   "name": "good-bayesian"
  },
  "language_info": {
   "codemirror_mode": {
    "name": "ipython",
    "version": 3
   },
   "file_extension": ".py",
   "mimetype": "text/x-python",
   "name": "python",
   "nbconvert_exporter": "python",
   "pygments_lexer": "ipython3",
   "version": "3.9.4"
  }
 },
 "nbformat": 4,
 "nbformat_minor": 5
}
(venv) marcogorelli@OVMG025 mcve % nbqa black Untitled.ipynb
All done! ✨ 🍰 ✨
1 file left unchanged.

From https://github.com/GoogleCloudPlatform/ai-platform-samples/blob/eed2ac7af3d552f16690c858439f22347a4d616e/.github/workflows/linter/requirements.txt it looks like you're on on an old version of nbQA

Please upgrade to the latest version (1.1.0) or use black directly (I contributed the PR psf/black#2357 , the logic is almost the same as nbQA's, it just works cell-by-cell)

If this still doesn't work for you with the latest version of nbqa, let me know and I'll reopen

@loferris
Copy link

loferris commented Sep 9, 2021

@MarcoGorelli With the linter as currently defined, the same error is occurring with the updated version of nbqa. I am now seeing about using black directly to see if this solves the problem.

@MarcoGorelli MarcoGorelli reopened this Sep 9, 2021
@MarcoGorelli
Copy link
Collaborator

Thanks @loferris - reopening for now then, I'll take another look

@loferris
Copy link

loferris commented Sep 9, 2021

when I run the latest version of black directly python3 -m black "$notebook" I get a runtime error and a prompt to open a bug report with nbQA

@MarcoGorelli
Copy link
Collaborator

I just ran both nbqa and black on the files which @ivanmkc mentioned, and got no error. Please

  1. show the output of:
nbqa --version
black --version
  1. share a notebook which reproduces the issue
  2. show the exact command you ran, along with the exact output

when I run the latest version of black directly python3 -m black "$notebook" I get a runtime error and a prompt to open a bug report with nbQA

There's no way that this could happen, black's codebase makes no mention of nbQA

@loferris
Copy link

loferris commented Sep 10, 2021

  1. nbqa --version was 0.6.0 on my machine (it's updated to 1.1.0 in the linter) but I upgraded it to 1.1.0 for the rest of this comment. black --version was 20.8b1 on my machine, upgraded to 21.8b0 for the rest of this comment. [This was because I didn't re-install the linter after changing the requirements .txt file.]
  2. Currently all of the notebooks are failing in this repo, with the same error, captured below. The exact problem notebook black wasn't able to parse before is here: https://github.com/GoogleCloudPlatform/bigquery-notebooks/blob/update_linter/notebooks/official/template_notebooks/bigquery_command_line_tool.ipynb
  3. Here's the code the linter runs:
if [ "$is_test" = true ] ; then
                echo "Running nbfmt..."
                python3 -m tensorflow_docs.tools.nbfmt --remove_outputs --test "$notebook"
                NBFMT_RTN=$?
                # echo "Running black..."
                # python3 -m nbqa black "$notebook" --check
                # BLACK_RTN=$?
                echo "Running pyupgrade..."
                python3 -m nbqa pyupgrade "$notebook"
                PYUPGRADE_RTN=$?
                echo "Running isort..."
                python3 -m nbqa isort "$notebook" --check
                ISORT_RTN=$?
                echo "Running flake8..."
                python3 -m nbqa flake8 "$notebook" --show-source --extend-ignore=W391,E501,F821,E402,F404,W503,W291,E203,E999,E111,E113 
                FLAKE8_RTN=$?
            else
                echo "Running black..."
                python3 -m black "$notebook"
                BLACK_RTN=$?            
                echo "Running pyupgrade..."
                python3 -m nbqa pyupgrade "$notebook"
                PYUPGRADE_RTN=$?
                echo "Running isort..."
                python3 -m nbqa isort "$notebook"
                ISORT_RTN=$?
                echo "Running nbfmt..."
                python3 -m tensorflow_docs.tools.nbfmt --remove_outputs "$notebook"
                NBFMT_RTN=$?
                echo "Running flake8..."
                python3 -m nbqa flake8 "$notebook" --show-source --extend-ignore=W391,E501,F821,E402,F404,W503,W291,E203,E999,E111,E113
                FLAKE8_RTN=$?                 
            fi

            NOTEBOOK_RTN="0"

(the file is here: https://github.com/GoogleCloudPlatform/bigquery-notebooks/blob/update_linter/.github/workflows/linter/run_linter.sh)
The output is captured here: black is failing the same way and the same error comes up for each subsequent invocation of nbqa.
Screen Shot 2021-09-10 at 4 08 03 PM

Oh, and you're right, I misread the original error! black was running, but then all following nbqa commands were failing with the JSONDecodeError

@loferris
Copy link

loferris commented Sep 10, 2021

When I go to that line in the .ipynb its questioning this:
'# Licensed under the Apache License, Version 2.0 (the "License");\n',
With the first character - ' - being l11,c17.
Manually changing around the quotes cleared that error, but there seems to be further JSON nits it gets bothered by.
Currently:

`nbQA failed to process bigquery_command_line_tool.ipynb with exception "JSONDecodeError('Expecting value: line 23 column 13 (char 1082)')"`
which is the closing bracket of `source`.
If you believe the notebook(s) to be valid, please report a bug at https://github.com/nbQA-dev/nbQA/issues

Another thing worth mentioning is that these odd changes seemed to have happened after I ran black (I think), as I haven't been manually messing around with the header code and previously it had double-quote formatting.

@MarcoGorelli
Copy link
Collaborator

MarcoGorelli commented Sep 11, 2021

OK, I see what's happened - you're missing the extra Jupyter requirements when running black on the notebooks, and so they're been formatted as if they were Python files. You probably ran black on the notebooks when the version was still 20.8b1, right?

If you want Jupyter support for black, you need to install it as

pip install -U black[jupyter]

I'd advise:

  • checkout your notebooks to the original state
  • pip install -U black[jupyter]
  • run black on the notebooks
  • run any other linting checks

The previous commit was fine btw (GoogleCloudPlatform/bigquery-notebooks@56712e9)

@MarcoGorelli
Copy link
Collaborator

As an aside, seeing as you're using nbQA at Google, I'd appreciate it if you could consider sponsoring the project

@MarcoGorelli
Copy link
Collaborator

HI @ivanmkc and @loferris - it looks like https://github.com/GoogleCloudPlatform/bigquery-notebooks/pull/18/files , so I gather this is now resolved?

Closing for now then, but please do let me know if you run into any other issues 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants