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

Refactor is_project_func method for Windows compatibility #1857

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

DraKen0009
Copy link
Contributor

The previous implementation of the is_project_func method had issues related to path handling on Windows systems, potentially leading to incorrect identification of project code.

Changes made:

  • Utilized os.path.normpath() for path normalization.
  • Modified file path comparison to correctly identify project code.
  • Implemented exclusion of 'site-packages' and 'dist-packages' directories.

Fixes: #1836

@tim-schilling
Copy link
Contributor

@DraKen0009 do you have a test that confirms that this fixes the problem on windows? I doubt we'll run it in CI, but it'll make me feel better if there were a test along with this.

@DraKen0009
Copy link
Contributor Author

DraKen0009 commented Nov 17, 2023

@tim-schilling , I haven't created test , but I can create a simple unit test for it, can you guide me where in project directory do I need to create it ?

@tim-schilling
Copy link
Contributor

It probably belongs in the test file for the profile panel. If you look at the tests directory, there should be a panels directory with files for each of the panels.

Copy link
Member

@matthiask matthiask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit saddened by the move from Path objects to os.path. Isn't there a way to achieve the same thing using pathlib?

@tim-schilling
Copy link
Contributor

@DraKen0009 is there any option to continue using pathlib?

@matthiask matthiask merged commit 6e9ce48 into jazzband:main Dec 18, 2023
26 checks passed
@matthiask
Copy link
Member

Tests pass and the implementation is strictly superior to the current one. I have removed the pathlib flake8 rules from my own projects since it's annoying and using pathlib isn't always better.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Profiler not correctly filtering files on Windows
3 participants