-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
isort
introduces bugs
#1653
Comments
Thanks for reporting, @erelsgl! @phschiele, can you look into this? |
@erelsgl The files you see differences in are exactly the known ones where the required ordering is conflicting with [tool.isort]
include_trailing_comma = true
use_parentheses = true
extend_skip = [
"cvxpy/__init__.py",
"cvxpy/reductions/__init__.py",
"cvxpy/reductions/dcp2cone/atom_canonicalizers/__init__.py"
] => The issue seems to be that your isort config is not picking this up. I've just tried
on a fresh clone, and for me it does work.
In particular, make sure that |
In
But still,
|
@erelsgl From which directory are you running |
isortcfg.txt
|
Do you have another which isort
python -m isort . You can also reach out via Discord, that might speed up debugging. |
which is the folder where I installed the virtual environment.
Is it possible that the problem is that the ignored files use "/" and the files on Windows use "\"? |
@erelsgl That is possible, but I would be surprised if that was the cause. You can try changing it locally, though. Can you alternatively try the following:
|
I changed the forward slashes to back slashes in
to
This does not cause any harm, but it is strange. On the other hand, |
@phschiele what do you think about closing this issue? To me, it seems like a bug in the usage / installation of |
@rileyjmurray Indeed seems like some issue with the local installation since the configuration clearly is being picked up for most users. Since the linters are usually considered towards the end of a PR, I don't think this would discourage someone from contributing, but I suppose adding a sentence to the docs wouldn't hurt. I can add this over the weekend. |
I suggest to not tell new developer to run Adding some info that the pre-commit will run |
I also just ran into this issue. Very weird. I just manually reverted changes. Here's the info on my isort install:
Here's my isort configuration for good measure.
|
@rileyjmurray I just set up a fresh codespace and only ran pip install isort
isort . and if worked, skipping 4 files. I also checked that there were no relevant differences in our configs (unfortunately the sorting was different in some places, so it could not compare line by line). Maybe you could post the results of |
@phschiele I'm pretty sure I figured it out. Here's what happens when I run isort in the same directory as CVXPY's setup.py file:
Meanwhile, here's what happens when I change directory to be level with
So it seems that the error can be attributed to isort seeing the configuration file based on the current working directory. |
Describe the bug
Following the CONTRIBUTING guide, I cloned the repository and ran
isort
. I expected it to not make any change, as the code is fresh from repository. However, three files were changed:After that, when I ran
I got an error:
ImportError: cannot import name 'InverseData' from partially initialized module 'cvxpy.reductions' (most likely due to a circular import)
.The error was gone after I reverted the changes made by
isort
.To Reproduce
clone the repository.
create a new virtual environment:
isort
:pytest
ontest_problem.py
:Expected behavior
isort
should not change any files.Output
Version
The text was updated successfully, but these errors were encountered: