add file argument #24
Conversation
|
This is now in conflict with the main branch. Could you rebase your commit and fix this ? |
|
done |
gepetuto/lint.py
Outdated
| folder = Path(f"tp{n}") | ||
| for python_file in folder.glob("*.py"): | ||
| lint_file(python_file) | ||
| for f in file: | ||
| lint_file(f) |
There was a problem hiding this comment.
The idea here is to filter files. Here you are adding new ones.
I guess we should iterate over file or glob for *.py when it is empty,
There was a problem hiding this comment.
you mean that there may be file that aren't .py ones and we should filter them ?
There was a problem hiding this comment.
I was not clear, sorry for that.
I mean that if we have the following structure:
├── 1_example_notebook.ipynb
├── 2_other_notebook.ipynb
├── tp1
│ ├── cholesky.py
│ └── example_script.py
└── tp2
├── cholesky.py
└── something.py
- with no tp_id and no file, we should find all 4 python files
- with tp_id=1 and no file, we should get the 2 python files in tp1
- with no tp_id and file=tp1/cholesky.py, we should get 1 python file in tp1
- with tp_id=1 and file=tp1/cholesky.py, we should get 1 python file in tp1
- with tp_id=2 and file=tp1/cholesky.py, we should get 0 python files
We'll also have to add -F / --filter to allow:
- with no tp_id and no file and filter=cholesky, we should get 2 python files, one in each tp
But that's for a separate future PR I think.
There was a problem hiding this comment.
no problem, understood
gepetuto/test.py
Outdated
| LOG.debug(f"Checking {python_file}") | ||
| check_call(["python", python_file]) | ||
| check_ipynb(n) | ||
| for f in file: | ||
| LOG.debug(f"Checking {f}") | ||
| check_call(["python", f]) |
gepetuto/lint.py
Outdated
| lint_file(python_file) | ||
| elif python_file in file: | ||
| lint_file(python_file) | ||
| if tp_id == []: |
There was a problem hiding this comment.
I don't see how this condition can be true. tp_id wil either be defined by the user to not the empty list, or not be defined by the user and auto-populated from get_tp_id()
There was a problem hiding this comment.
if someone has no tp folder and doesn't specify tp_id it could happen but i agree it shouldn't, should I still remove it ?
There was a problem hiding this comment.
Yes, I think the logic between this if and its previous block should be changed. Probably we don't want to glob on *.py if -f is provided.
TheoMF
force-pushed
the
script_args_#10
branch
4 times, most recently
from
ea76c66
to
6ddcfa6
Compare
| @@ -7,6 +7,7 @@ repos: | |||
| rev: v0.0.281 | |||
| hooks: | |||
| - id: ruff | |||
| exclude: gepetuto/magic.py | |||
There was a problem hiding this comment.
This won't prevent manual runs of ruff . to fail. I think you should instead add hints in the file : either a # noqa: F821 in the end of each line where there is a get_ipython, or a # ruff: noqa: F821 at the begining of the file.
A better solution might be to add a def run(self, magic) method, which run get_ipython().run_line_magic(magic) with the correct noqa only once, and use this method in the other places.
| LOG.debug(f"Checking {python_file}") | ||
| check_call([python_interpreter, python_file]) | ||
| check_ipynb(n, python_interpreter) | ||
| tp_id = int(str(files[0])[2]) # get the tp id of the first file |
There was a problem hiding this comment.
Ok, we'll probably want to move to a {tp_number: [list of files in this tp]} structure at some point, but this is fine for now
fix #10