ploomber / jupysql

Better SQL in Jupyter. 📊
https://jupysql.ploomber.io
Apache License 2.0
712 stars 76 forks source link

confusing error when executing `%sqlcmd` #262

Closed edublancas closed 1 year ago

edublancas commented 1 year ago

When executing %sqlcmd, I get this:

Screenshot 2023-03-16 at 6 44 33 p m

But the error is unclear. Instead, we should throw an UsageError and tell the user what are the available commands, along with some examples

raghavSharmaCode commented 1 year ago

Request to assign this issue (#262)

raghavSharmaCode commented 1 year ago

Hi Ido/ Eduardo,

I would like to know the control flow of the above command '%sqlcmd'.

I have forked the repo and created a new branch 'dev-262'. I changed the file magic_cmd.py to debug the issue. The issue occurs at line 44. So after line 43, I added some print statements to check the split of the line argument.

However, when I committed the file and tried to install the repo as a pip package using:

(pip install git+https://github.com/raghavSharmaCode/jupysql.git@dev-262)

It did not reflect the changes that I was expecting when I ran the '%sqlcmd' command in a test .ipynb file.

raghavSharmaCode commented 1 year ago

Hence I would like to learn about the OOPS implementation of this package so that we are able to debug and resolve the issue on a low-level design basis.

edublancas commented 1 year ago

did you setup with the invoke setup command? Once you fork and clone the repo, you can run:

invoke setup

This will install JupySQL in editable mode, meaning all changes should reflect (if you're using Jupyter, you need to enable autoreload or restart the kernel, though)

To check if JupySQL is correctly installed in the current environment:

In [1]: import sql; sql
Out[1]: <module 'sql' from '/Users/eduardo/dev/jupysql/src/sql/__init__.py'>

You can see that sql prints the path where I cloned the repo, you should see something similar

raghavSharmaCode commented 1 year ago

Hi Eduardo,

Please confirm if the below image explains what you asked for.

Ploomber issue #262

The latest code is in the dev-262 repo.

Please confirm if I should raise a pull request for you to verify.

Thanks and regards, Raghav

edublancas commented 1 year ago

Looks good!

Some feebdback:

If the user passes

%sqlcmd

The error should say:

Missing argument for %sqlcmd
Valid commands are: tables, columns

If they pass a wrong argument:

%sqlcmd stuff

The error should say:

stuff is not a valid argument for %sqlcmd
Valid commands are: tables, columns
raghavSharmaCode commented 1 year ago

As requested,

#262-final

Also, I would like to know the significance of the default line argument as an empty string. An empty string is a use case but how much is it relevant as the default argument?

I will be raising a pull request for you to verify the changes. Looking forward to your response.

Best, Raghav

edublancas commented 1 year ago

for reference, the PR is: https://github.com/ploomber/jupysql/pull/271