feat: Added one of the isort rule/check to ruff

[Sai-Suraj-27]

PR Description

Added the isort rule/check to ruff. Configured it properly in pyproject.toml to resolve conflicts with the ivy_lint

Related Issue

Closes #

Checklist

• Did you add a function?
• Did you add the tests?
• Did you run your tests and are your tests passing?
• Did pre-commit not fail on any check?
• Did you follow the steps we provided?

Socials

[KareemMAX]

Per our private discussion:

For some reason, all the checks for the isort PR are failing. Due to some import error, as we rearranged all the imports. Please tell me after you look into it. So, that we can try and fix the problem.

ImportError while loading conftest 'D:\repos\ivy_latest\ivy\ivy_tests\test_ivy\conftest.py'.
ivy_tests\test_ivy\conftest.py:12: in <module>
    from .helpers.globals import mod_backend, mod_frontend
ivy_tests\test_ivy\helpers\__init__.py:3: in <module>
    from . import assertions, function_testing, hypothesis_helpers, testing_helpers
ivy_tests\test_ivy\helpers\assertions.py:3: in <module>
    import ivy
ivy\__init__.py:764: in <module>
    from . import func_wrapper, functional, stateful
ivy\functional\__init__.py:1: in <module>
    from . import ivy
ivy\functional\ivy\__init__.py:3: in <module>
    from . import (
ivy\functional\ivy\creation.py:22: in <module>
    from ivy import to_ivy
E   ImportError: cannot import name 'to_ivy' from partially initialized module 'ivy' (most likely due to a circular import) (D:\repos\ivy_latest\ivy\ivy\__init__.py)

Edit: I tested directly with isort pre-commit hook instead of adding it through ruff but the same ImportError is still coming. So, this is not some issue due to improper working of isort tool in ruff.

I would say it's better to not merge these changes, maybe you can liaise with @vedpatwardhan to see how this issue might be fixed. But I wouldn't give it much thought sense it's really a quality of life improvement rather than an important addition

[vedpatwardhan]

Per our private discussion:

For some reason, all the checks for the isort PR are failing. Due to some import error, as we rearranged all the imports. Please tell me after you look into it. So, that we can try and fix the problem.

ImportError while loading conftest 'D:\repos\ivy_latest\ivy\ivy_tests\test_ivy\conftest.py'.
ivy_tests\test_ivy\conftest.py:12: in <module>
    from .helpers.globals import mod_backend, mod_frontend
ivy_tests\test_ivy\helpers\__init__.py:3: in <module>
    from . import assertions, function_testing, hypothesis_helpers, testing_helpers
ivy_tests\test_ivy\helpers\assertions.py:3: in <module>
    import ivy
ivy\__init__.py:764: in <module>
    from . import func_wrapper, functional, stateful
ivy\functional\__init__.py:1: in <module>
    from . import ivy
ivy\functional\ivy\__init__.py:3: in <module>
    from . import (
ivy\functional\ivy\creation.py:22: in <module>
    from ivy import to_ivy
E   ImportError: cannot import name 'to_ivy' from partially initialized module 'ivy' (most likely due to a circular import) (D:\repos\ivy_latest\ivy\ivy\__init__.py)

Edit: I tested directly with isort pre-commit hook instead of adding it through ruff but the same ImportError is still coming. So, this is not some issue due to improper working of isort tool in ruff.

I would say it's better to not merge these changes, maybe you can liaise with @vedpatwardhan to see how this issue might be fixed. But I wouldn't give it much thought sense it's really a quality of life improvement rather than an important addition

Yeah I think in that case, probably for the best to hold off on this formatting for now (also considering we had done a major formatting of the repo a couple of months back anyway), thanks for creating the PR though @Sai-Suraj-27 😄

[vedpatwardhan]

not sure why the ivy import is considered a global import here but not in the other files like this. The same seems to exist in a few other files as well 😅
I'll leave this with @KareemMAX to make the final decision on whether or not we need this, but there's a few issues like this which need fixing if we are to merge this PR. Thanks @Sai-Suraj-27 😄

[KareemMAX]

I think I did arrive here a bit late 😅. I think this change is no longer being made

[Sai-Suraj-27]

Hey, @KareemMAX, @vedpatwardhan I don't know the exact reason for the failing of tests (I'm guessing it is due to the presence of circular imports). So, the isort in ruff provides 2 rules:

1. To sort the imports
2. Report the missing required imports.

So, I have modified the PR to add only the second check, as sorting the imports as per the 1st check is introducing a lot of failures.

[vedpatwardhan]

Hey, @KareemMAX, @vedpatwardhan I don't know the exact reason for the failing of tests (I'm guessing it is due to the presence of circular imports). So, the isort in ruff provides 2 rules:

1. To sort the imports
2. Report the missing required imports.

So, I have modified the PR to add only the second check, as sorting the imports as per the 1st check is introducing a lot of failures.

Do we need to add this check @NripeshN? I suppose we were already doing some sort of sorting in the gardener right? Was that just for functions or for imports too? Thanks for bringing that up @Sai-Suraj-27 😄

[github-actions]

PR Compliance Checks

Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.

Issue Reference

In order to be considered for merging, the pull request description must refer to a specific issue number. This is described in our contributing guide and our PR template.
This check is looking for a phrase similar to: "Fixes #XYZ" or "Resolves #XYZ" where XYZ is the issue number that this PR is meant to address.

[NripeshN]

Hey, @KareemMAX, @vedpatwardhan I don't know the exact reason for the failing of tests (I'm guessing it is due to the presence of circular imports). So, the isort in ruff provides 2 rules:

1. To sort the imports
2. Report the missing required imports.

So, I have modified the PR to add only the second check, as sorting the imports as per the 1st check is introducing a lot of failures.

Do we need to add this check @NripeshN? I suppose we were already doing some sort of sorting in the gardener right? Was that just for functions or for imports too? Thanks for bringing that up @Sai-Suraj-27 😄

The ivy gardener does not sort imports like how ruff does. It does do some sort of sorting on imports
- If the node represents an import statement, it receives the highest priority.
- For try-except blocks containing imports, they are considered next.

[vedpatwardhan]

The ivy gardener does not sort imports like how ruff does. It does do some sort of sorting on imports - If the node represents an import statement, it receives the highest priority. - For try-except blocks containing imports, they are considered next.

Got it, so do you think the changes made to the pyproject.toml in this PR are a useful addition? If yes, we can merge this PR. If not, we'd have to close the PR. Thanks @NripeshN @Sai-Suraj-27 😄

[Sai-Suraj-27]

Hey, @NripeshN just to be clear. I have added one of the checks from isort related checks of ruff that will report any Missing required imports in a file.

[NripeshN]

The ivy gardener does not sort imports like how ruff does. It does do some sort of sorting on imports - If the node represents an import statement, it receives the highest priority. - For try-except blocks containing imports, they are considered next.

Got it, so do you think the changes made to the pyproject.toml in this PR are a useful addition? If yes, we can merge this PR. If not, we'd have to close the PR. Thanks @NripeshN @Sai-Suraj-27 😄

Yup,
I think this would be a nice addition
Thanks @vedpatwardhan @Sai-Suraj-27

[vedpatwardhan]

awesome, feel free to merge the PR @he11owthere, thanks @Sai-Suraj-27 😄