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

--show-source code frames should respect Jupyter cells #5395

Closed
charliermarsh opened this issue Jun 27, 2023 · 2 comments · Fixed by #5402
Closed

--show-source code frames should respect Jupyter cells #5395

charliermarsh opened this issue Jun 27, 2023 · 2 comments · Fixed by #5402
Assignees
Labels
bug Something isn't working

Comments

@charliermarsh
Copy link
Member

I have two cells, one containing two imports, and another containing a function definition.

When I run cargo run -p ruff_cli -- check Untitled.ipynb -n --watch --show-source, I get:

Untitled.ipynb:cell 1:2:8: F401 [*] `sys` imported but unused
  |
1 | import os
2 | import sys
  |        ^^^ F401
3 | def f(x):
4 |     y = x * x
  |
  = help: Remove unused import: `sys`

Untitled.ipynb:cell 2:2:5: F841 [*] Local variable `y` is assigned to but never used
  |
2 | import sys
3 | def f(x):
4 |     y = x * x
  |     ^ F841
5 |     print(x * x)
6 | f(2)
  |
  = help: Remove assignment to unused variable `y`

I think the code frames need to be reindexed. Notice that in the second error, the location is reported as cell 2:2:5, but in the code frame, the violation is on line 4.

Similarly, we should omit any contents from adjoining cells when displaying the code frame (in my opinion) -- so the second diagnostic shouldn't show the import sys.

@charliermarsh
Copy link
Member Author

\cc @dhruvmanila - Wdyt?

@charliermarsh charliermarsh added the bug Something isn't working label Jun 27, 2023
@dhruvmanila
Copy link
Member

Ah yes, this is true. I'll need to look into how source is being shown currently. Most likely the cell separation concept could be common for --show-source and --diff option.

@dhruvmanila dhruvmanila self-assigned this Jun 27, 2023
dhruvmanila added a commit that referenced this issue Jun 28, 2023
## Summary

Consider Jupyter index for code frames (`--show-source`).

This solves two problems as mentioned in the linked issue:

> Omit any contents from adjoining cells

If the Jupyter index is present, we'll use that to check if the
surrounding
lines belong to the same cell as the content line. If not, we'll skip
that line
until we either reach the one which does or we reach the content line.

> code frame line number

If the Jupyter index is present, we'll use that to get the actual start
line in
corresponding to the computed start index.

## Test Plan

`cargo run --bin ruff -- check --no-cache --isolated --select=ALL --show-source /path/to/notebook.ipynb`

fixes: #5395
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants