chore: more pre-commit checking#2257
Conversation
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
henryiii
force-pushed
the
henryiii/chore/precom2
branch
from
d479ead to
7531cc5
Compare
| try: | ||
| return "".join(traceback.format_exception_only(type(e), e)) | ||
| finally: | ||
| e = None # type: ignore |
There was a problem hiding this comment.
Ah I guess this isn't needed since e wasn't thrown down the stack from this function, so e's traceback won't contain a reference to this scope hence no reference loop to break.
There was a problem hiding this comment.
If it does cause a problem, I think you want del e rather than e = None.
There was a problem hiding this comment.
I copied this from the Python source code. Which doesn't mean that it is the right way to do things of course. It seems that in the CPython source code for v3.10.2 there are 32 instances of finally: some_var = None and 36 instances of finally: del some_var.
There was a problem hiding this comment.
The Python source code is not type checked, so there's no strong difference between x = None and del x. But with type checking, there is a strong differentiator - the former requires x to have an Optional type, which complicates all other usages. While the del does not.
|
|
||
| gunzip -rf f2c/* | ||
|
|
||
| cd f2c |
There was a problem hiding this comment.
I'm not convinced we should have this file in here at all. Again, it looks like it's specifically for OSX. Is there really no better way to install f2c on osx than using this script? I guess having an untested and possibly slightly broken script is better than none?
There was a problem hiding this comment.
Maybe it could be added to homebrew? I see it in macPorts.
There was a problem hiding this comment.
Well I certainly won't do that, but it would be great if someone else would. In the meantime leaving this unmaintained script here is less work than fixing the problem.
There was a problem hiding this comment.
I'd do it if it wasn't so annoying. No versioned link to the files AFAICT, and brew's not going to like that. Barely even versions (dates, I think).
|
We really need to get rid of the line number in |
src/tests/test_console.py
Outdated
| >>> Test() | ||
| [[;;;terminal-error]Traceback (most recent call last): | ||
| File \"/lib/{PYVERSION}/site-packages/_pyodide/console.py\", line 465, in repr_shorten | ||
| File \"/lib/{PYVERSION}/site-packages/_pyodide/console.py\", line """ |
There was a problem hiding this comment.
I was thinking of something like result = re.sub("line [0-9]*", "line xxx", result) but I suppose this gets the job done too.
There was a problem hiding this comment.
I think doing the regex substitution is preferable because assert x == y gives better pytest error messages than assert x.endswith(y).
Signed-off-by: Henry Schreiner <[email protected]>
henryiii
force-pushed
the
henryiii/chore/precom2
branch
from
e64d93c to
4d3fad5
Compare
2 participants
This cleans up the flake8 & mypy checks, adds a bit of spell checking, and a linter for shell scripts.
python/typeshed#7453 will help, along with python/typeshed#7452 (already fixed in typeshed).