Skip to content

Only use translated pkg name as fallback + some optimization#337

Merged
fpgmaas merged 3 commits intofpgmaas:mainfrom
akeeman:patch-2
May 6, 2023
Merged

Only use translated pkg name as fallback + some optimization#337
fpgmaas merged 3 commits intofpgmaas:mainfrom
akeeman:patch-2

Conversation

@akeeman
Copy link
Contributor

@akeeman akeeman commented Apr 20, 2023

PR Checklist

  • A description of the changes is added to the description of this PR.
  • If there is a related issue, make sure it is linked to this PR.
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

Description of changes
It should have been split across multiple PRs, but I'm trying it like this anyway:

  • The translated package name is now only added to the recognized top level modules if no mapping or metadata is found.
  • If a metadata file is found, but it's empty, no alternative methods of finding top level module names will be tried. Previously no distinction was made between not found and found but empty.
  • optimized the code a bit to reduce the up to three metadata.distribution(name) calls to one.
  • recognize more top level module names in RECORDS file ("foo.py,..." is now recognized as "foo", in accordance to the top_level.txt provided names)
  • remove "__pycache__" from recognized names in RECORDS file

🚨 not adding the translated package name by default may, but in practice should not, break things.
🚨 removed public instance method Dependency::find_metadata

In the test file you'll find changes where package "foo" has been renamed to "qux". This is because it was not meant to collide with a package "foo" defined in tests/data/project_with_src_directory, but does, while that package has no top level module "foo". Due to the translated package name no longer being added, this now resulted "foo" missing in the top level module names.

Context
fixes #335
fixes #336

@fpgmaas
Copy link
Owner

fpgmaas commented May 3, 2023

@akeeman I think the solution direction looks good! Do you want to resolve the conflict with the recent addition you made to dependency.py? If not, I am also happy to pick this up soon.

@akeeman
Copy link
Contributor Author

akeeman commented May 3, 2023

Sure, let me fix it. I just wanted to know your thoughts before putting in more work.

@akeeman akeeman changed the title Only use the translated package name as a fallback when no metadata i… Only use the translated package name as a fallback May 4, 2023
@codecov
Copy link

codecov bot commented May 4, 2023

Codecov Report

Merging #337 (4a78fc2) into main (cacc0a1) will increase coverage by 0.2%.
The diff coverage is 93.9%.

@@           Coverage Diff           @@
##            main    #337     +/-   ##
=======================================
+ Coverage   96.0%   96.2%   +0.2%     
=======================================
  Files         29      29             
  Lines        941     938      -3     
  Branches     192     195      +3     
=======================================
- Hits         904     903      -1     
+ Misses        24      22      -2     
  Partials      13      13             
Impacted Files Coverage Δ
deptry/dependency.py 94.7% <93.9%> (+3.0%) ⬆️

@akeeman akeeman changed the title Only use the translated package name as a fallback Only use translated pkg name as fallback + some optimization May 4, 2023
@akeeman akeeman marked this pull request as ready for review May 4, 2023 14:44
Copy link
Collaborator

@mkniewallner mkniewallner left a comment

Choose a reason for hiding this comment

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

Looks like a welcome change, thanks for the contribution.

@mkniewallner mkniewallner mentioned this pull request May 5, 2023
@mkniewallner mkniewallner added this to the 0.9 milestone May 5, 2023
@fpgmaas fpgmaas self-requested a review May 6, 2023 08:19
@fpgmaas fpgmaas merged commit c81422a into fpgmaas:main May 6, 2023
@akeeman akeeman deleted the patch-2 branch May 6, 2023 15:20
@mkniewallner mkniewallner added the breaking Change that introduces a breaking change label May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Change that introduces a breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deptry could lookup metedata only once Deptry should not always add translated package name as module name

3 participants