bndr / pipreqs

pipreqs - Generate pip requirements.txt file based on imports of any project. Looking for maintainers to move this project forward.
Apache License 2.0
6.38k stars 388 forks source link

Incorret package included for Hydra #244

Closed sxooler closed 3 years ago

sxooler commented 3 years ago

I'm using Facebook hydra-core package (see Hydra.cc). But this package is not correctly handled because of another package with the same name (hydra - the python bloom filter).

Example

# content of my.py, the only file in my dir to show the behavior with pipreqs
import hydra

if __name__ == "__main__":
    print(hydra.__version__)

Case 1:

Only hydra-core is locally installed

# Name Version Build Channel hydra-core 1.0.6 pypi_0 pypi

Then output of pipreqs --print is:

hydra_core==1.0.6
hydra==2.5
INFO: Successfully output requirements

And output of pipreqs --print --use-local is:

hydra_core==1.0.6
INFO: Successfully output requirements

Case 2:

When both hydra-core and hydra are installed (this easily happens to anyone using fb hydra-core package because of accidentally using pip install hydra instead of pip install hydra-core), then for both commands (with and without --use-local flag) the output is following:

Hydra==2.5
INFO: Successfully output requirements

Question

I was thinking about updating mapping with a line of hydra:hydra-core (similarly to https://github.com/bndr/pipreqs/pull/234). Is that the right way how to deal with this issue? I was just a bit wondering about this since the behavior described above was quite strange to me (I was also confused with e.g. why hydra starts sometimes with capital letter and sometimes not).

Environment

Python 3.8.6 pipreqs 0.4.10

alan-barzilay commented 3 years ago

hi! that's an interesting conundrum, my intuition is that by adding a mapping like that to hydra-core we would just shift the problem, we would be breaking it for users that want hydra and not hydra-core.

To import hydra-core you just import hydra, correct? How does one import the hydra package? And these 2 packages are completely unrelated?

alan-barzilay commented 3 years ago

(I was also confused with e.g. why hydra starts sometimes with capital letter and sometimes not).

not sure what you meant by that, but this may be related: https://github.com/pypa/setuptools/blob/9755c3bc2bf1fe50bf67dccb583ffad9d57f7bab/pkg_resources/__init__.py#L1312 This function is used to make package names in Pypi adhere to package naming conventions, which include lowering the case of everything

omry commented 3 years ago

Hi @alan-barzilay, Here is some additional context on the issue.

In particular, I think the download graphs and the number of public GitHub dependencies for each project should be a good enough justification to prefer hydra-core over hydra as the package associated with the import hydra. hydra-core is already much more popular than hydra, even though it's a much younger project.

alan-barzilay commented 3 years ago

Ok, I'm sold. This is not a real solution by any means but I'm ok with it, if you make a PR I will merge it (just be careful as to not invert the mapping). But, just a heads-up, once it is merged you will have to install pipreqs without pip since there won't be an immediate new release

omry commented 3 years ago

Yeah, a more sophisticated solution is to analyze the actually used modules from hydra and determine which package is more likely. This seems beyond the current capabilities of pipreqs at the moment though.

omry commented 3 years ago

@sxooler, want send a PR?

sxooler commented 3 years ago

(I was also confused with e.g. why hydra starts sometimes with capital letter and sometimes not).

not sure what you meant by that, but this may be related: https://github.com/pypa/setuptools/blob/9755c3bc2bf1fe50bf67dccb583ffad9d57f7bab/pkg_resources/__init__.py#L1312 This function is used to make package names in Pypi adhere to package naming conventions, which include lowering the case of everything

When you check my question, regarding Hydra (the other package I'm not interested in) there is difference in the output between case 1 and case 2. In the first one, output is with lowercase (hydra==2.5), in the second it starts with capital (Hydra==2.5). You might be right, I was just not sure why this is happening.

@sxooler, want send a PR?

Sorry for late reply, yes, I will send it next week.