isort Module is Observed in /spotipy/*

[lnxpy]

I installed the isort module right beside the other dependencies. Now, isort is observed in /spotipy/*.py files. There is no big change in the main algorithm.

I'm contributing in Hacktoberfest challenge. Be sure that I'll make some essential changes in the project. I just need to add the "hacktoberfest" label in this PR.

Thanks.

[stephanebruckert]

Thanks for that. Please update the changelog :-)

Also, how does one use "isort"? is there a command we can use to sort imports in the future? could we add this as a CI check?

[lnxpy]

Thanks for your responsing. I'm working on the logging side of the Spotipy now. (using the default script-based configurations)
Be sure, I'll update the changelog file.

I do guess it brings some troubles in using isort module in the CI services. I've worked on a lot of projects and used isort in order to sort the import lists but in some cases, it just failed in tests!! (e.g. you can take a look at this closed PR on IBM/lale)

Finally, I enthusiastically appreciate you for adding the label to the PR. Thanks.

[stephanebruckert]

We could merge your PR as it is, but it would be better if others can profit from it:

1. what command did you run? add it to https://github.com/plamere/spotipy/blob/master/CONTRIBUTING.md#lint

2. in the case of our CI, we could probably run a command that checks if imports are sorted or not, see https://github.com/PyCQA/isort#the---check-only-option. So maybe, after the line that checks if the code is formatted https://github.com/plamere/spotipy/blob/88ad7e05d515740844c7f654f81f4fddf3e517fa/.github/workflows/pythonapp.yml#L27 add the command you ran in (1) by adding -c to it

3. As shown in https://github.com/PyCQA/isort#setuptools-integration, I'm thinking we should add isort to the spotipy extra_reqs, so that everyone working on spotipy will automatically install it: https://github.com/plamere/spotipy/blob/88ad7e05d515740844c7f654f81f4fddf3e517fa/setup.py#L14

[stephanebruckert]

extra line to remove

[stephanebruckert]

You can remove this line as 2.16.0 was already released and your changes are still unreleased

[lnxpy]

Everything is ok with these changes. Flake has restored some changes in the scripts which are completely compatible in isort. Hope it's ready to be merged.

[lnxpy]

I suggest you add the flake8 plugin instead of using isort. It helps you to verify if all import lists are sorted. Check flake8-isort module. (I've never worked with this tool and that why I have no idea about editing the lint section so I leave it on your own)

Thank you so much. :)

[stephanebruckert]

I've never worked with this tool and that why I have no idea about editing the lint section so I leave it on your own

You have to start sometime @lnxpy 😺

Your PR in its current state does not break anything so that's great, but it also doesn't add long-term value. Codebase changes should come with tests and changing the style of a codebase without adding an automatic rule is useful only temporarily because others won't follow it and soon, your initial sorting will be erased. You probably wouldn't like to open that same PR in a month, would you?

For your PR to get the value it deserves and be merged, it would need:

• instructions for other contributors to do the same, so they don't have to look for it and we won't loose your initial investment (please don't do this),
• even better, an automatic rule so that we don't need to worry about it ever again (please do this).

I suggest you add the flake8 plugin instead of using isort

I also agree that using the flake8-isort plugin is better than using isort itself, because we already use flake8. Please note that there is also https://pypi.org/project/flake8-import-order/, feel free to pick your favorite

[lnxpy]

Thanks @stephanebruckert,
I'm kinda confused now. 😅 Where should I sign? Is there any other file to change and submit these changes? (Because I've already changed the changelog file)

[stephanebruckert]

@lnxpy that's fine, what command did you run to sort the imports? please add that command to the following doc, so that next contributors know how to do it: https://github.com/plamere/spotipy/blob/master/CONTRIBUTING.md#lint and we can merge your PR

[lnxpy]

I've already worked on the logging system. I suppose to make another PR for those changes.
Thanks again. Such a long conversation!! :D

[stephanebruckert]

Yeah I just wanted to express how important it is to share your knowledge, which you did, so thank you. Looking forward to your next PR!