Skip to content
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

Pylint is not using a src/ folder #5644

Open
CarliJoy opened this issue Jan 6, 2022 · 9 comments
Open

Pylint is not using a src/ folder #5644

CarliJoy opened this issue Jan 6, 2022 · 9 comments
Labels
Enhancement ✨ Improvement to a component Maintenance Discussion or action around maintaining pylint or the dev workflow Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@CarliJoy
Copy link
Contributor

CarliJoy commented Jan 6, 2022

Bug description

Pylint does not have a src folder

Led me into problems developing and test at least from time to time.

Here an exmplaination stolen why this a good idea:

Why does PyScaffold ≥ 3 have a src folder which holds the actual Python package?

This avoids quite many problems compared to the case when the actual Python package resides in the same folder as
your configuration and test files.
A nice blog post by Ionel gives a thorough explanation why this is so. In a nutshell, the most severe
problem comes from the fact that Python imports a package by first looking at the current working directory and then
into the PYTHONPATH environment variable. If your current working directory is the root of your project directory
you are thus not testing the installation of your package but the local package directly. Eventually, this always
leads to huge confusion ("But the unit tests ran perfectly on my machine!").

Moreover, having a dedicated src directory to store the package files, makes it easy to comply with recent standards
in the Python community (for example PEP 420).

Please notice that PyScaffold assumes all the files inside src are meant to be part of the package.

Configuration

No response

Command used

cd <other pylint dev folder> && pytest

Pylint output

some strange stuff, as pytests test the pylint in the other folder while I have also pylint installed as -e from the other -venv

Expected behavior

no problem because src folder prevents them

@CarliJoy CarliJoy added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jan 6, 2022
@Pierre-Sassoulas Pierre-Sassoulas added Discussion 🤔 Proposal 📨 and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jan 6, 2022
@Pierre-Sassoulas Pierre-Sassoulas added the Needs decision 🔒 Needs a decision before implemention or rejection label Jul 5, 2022
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.15.0 milestone Jul 5, 2022
@DanielNoord
Copy link
Collaborator

I'm not really a big fan of moving everything around as it will screw up git blame. As far as I know we haven't been having any issues with our project structure recently so without any immediate benefit I only see potential downsides here.

-0.5 for me.

@jacobtylerwalls
Copy link
Member

Same. Major projects like django do not use a src folder. Thanks for the suggestion, it was worth having a look 👍🏻

@jacobtylerwalls jacobtylerwalls closed this as not planned Won't fix, can't repro, duplicate, stale Aug 25, 2022
@jacobtylerwalls jacobtylerwalls removed the Needs decision 🔒 Needs a decision before implemention or rejection label Aug 25, 2022
@jacobtylerwalls jacobtylerwalls removed this from the 2.15.0 milestone Aug 25, 2022
@CarliJoy
Copy link
Contributor Author

CarliJoy commented Oct 6, 2022

I'm not really a big fan of moving everything around as it will screw up git blame. As far as I know we haven't been having any issues with our project structure recently so without any immediate benefit I only see potential downsides here.

-0.5 for me.

FYI @DanielNoord the git blame issue can be easily solved with https://akrabat.com/ignoring-revisions-with-git-blame/

Also git improved quite a bit with the latest version, so renames are detected much better.

Just to document this.

@jacobtylerwalls
Copy link
Member

After #8902 and the number of hours sunk into investigating subtle issues, I'm now a fan of doing this. We should at least have a discussion about it.

@jacobtylerwalls jacobtylerwalls added Needs decision 🔒 Needs a decision before implemention or rejection and removed Won't fix/not planned labels Aug 5, 2023
@DanielNoord
Copy link
Collaborator

I'm fine with it if we double check that git blame still works. If that's the case, let's do it!

@Pierre-Sassoulas
Copy link
Member

I'm not worried about git blame being broken, the imports should not change, so even small files with import only should be correctly detected as moved not deleted. I'm not using the SRC folder structure in any of my project but I'll get used to it :D

@DanielNoord
Copy link
Collaborator

I'm also a fan of doing this after having everything come crashing down at my day job because of issues related to this.

Let's do this for the next major release. @CarliJoy Are you still interested in pursuing this and potentially creating a PR? All maintainers seem aligned.

@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component Maintenance Discussion or action around maintaining pylint or the dev workflow Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Proposal 📨 Discussion 🤔 Needs decision 🔒 Needs a decision before implemention or rejection labels Nov 14, 2024
@CarliJoy
Copy link
Contributor Author

@DanielNoord I am sorry I fell victim to ruff. So as I am not using pylint anymore. Therefore I am rather investing my time in other issues i.e. into the python typing ecosystem.

But I am happy to help if there are any troubles arising from the migration.

@DanielNoord
Copy link
Collaborator

That's understandable. I don't think I have time to pick this up myself, but I'd like to thank you for making the original issue :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Maintenance Discussion or action around maintaining pylint or the dev workflow Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

4 participants