thebjorn / pydeps

Python Module Dependency graphs
https://pydeps.readthedocs.io/en/latest/
BSD 2-Clause "Simplified" License
1.8k stars 114 forks source link

stdlib-list only for python < 3.10 #190

Closed thomas-brandstaetter closed 1 year ago

thomas-brandstaetter commented 1 year ago

• modify requirements to import stdlib-list when python version < 3.10

thebjorn commented 1 year ago

A better solution would be top remove the special casing at https://github.com/thebjorn/pydeps/blob/master/pydeps/pystdlib.py#L13 and just use the 0.9.0 version of stdlib-list and/or implement the suggestions in https://github.com/thebjorn/pydeps/issues/171

thomas-brandstaetter commented 1 year ago

I prefer not to include an unnecessary, deprecated library if it's not needed, isn't used, and shouldn't even be used. It causes waste of space in disk and memory, may cause incompatibilities with libraries stating stdlib_list for an exact or minor version than 0.9.0.

My concern was that pip install ... installed a dependency which is useless. Since PEP8 gives no advice for conditional imports and the if foobar: idiom is widely used, i think thats a proper solution.

thomas-brandstaetter commented 1 year ago

I have a little concern about having three places asking for python version to decide whether include stdlib_list or not. I would put them into a central place defining two expressions. One for defining the max python version and another one for storing a boolean sys.version_info[:2] < (3, xxx)

thebjorn commented 1 year ago

stdlib_list is no longer deprecated, it has been transferred to the pypa team :-)

thebjorn commented 1 year ago

Thanks, but the fundamental problem with the current workaround:

def pystdlib():
    """Return a set of all module-names in the Python standard library.
    """
    if sys.version_info[:2] >= (3, 10):
        # Python 3.10 has this functionality built-in.
        return list(sys.stdlib_module_names | set(sys.builtin_module_names))
    ...

is that it doesn't contain the full hierarchy of names (e.g. py3.11.3):

>>> import sys
>>> from stdlib_list import stdlib_list
>>> len(stdlib_list())
1027
>>> len(sys.stdlib_module_names | set(sys.builtin_module_names))
307
>>>

so the solution is to remove the workaround (especially now that stdlib-list is maintained by the pip-maintainers).

thomas-brandstaetter commented 1 year ago

I investigated some time in this. The difference between

list(sys.stdlib_module_names | set(sys.builtin_module_names))

and

stdlib_list.stdlib_list

is, that the latter includes submodules recursively which are not included in the first approach.

Since the work is done by stdlib_list, this pull request is outdated

thomas-brandstaetter commented 1 year ago

But, i want to remind you that your current implementation has 1 item more in the list than stdlib_list. __main__ is in your list but not in stdlib_list for_sharing.py.txt

My best guess is that __main__ comes from the interpreter like ipython and consorts.

Code supposing you are on unix.

It is not quite hard to make such a list yourself with importlib and inspect. But it's not easy to navigate through poor documented return types and take care of loops.

thomas-brandstaetter commented 1 year ago

stdlib_list already achieves what this pull request achieves.