Feature/no import third party provided by stdlib

[Spitfire1900]

• Do not install tomli on python>=3.11
• Do not install importlib-metadata on python>=3.8
• Remove try/catch blocks as tomi/tomllib is guaranteed by package requirements
• Remove unused module imports

[Spitfire1900]

Supersedes #1152

[char101]

Using metadata prevents the module from being run by simply being added to PYTHONPATH. I think the metadata code should be wrapped in try/except because the module works fine without the metadata.

[eli-schwartz]

But that isn't changed by this PR, so it was a problem all along (since #1067).

The reason originally given for using importlib_metadata was to avoid circular imports of some sort? Which seems odd but I never dug into the history.

[Spitfire1900]

@char101 , platformdirs suffers from the same situation.

@bwendling should yapf support being run without any dependencies? Does that warrant any additional complexity in the application logic?

[eli-schwartz]

I'm not sure that's what is being requested here, to be fair.

The issue is that if you've manually installed the dependencies, and the project in question doesn't have any fancy build steps but simply installs the contents of {name}/ as-is, then it's plausible one could simply run the project from the source tree.

But using importlib.metadata adds a new build step. Suddenly you have to generate an egg-info or dist-info before yapf can run.

I'm not sure how generally useful it is to be able to do that. Probably the main reason I can think of off the top of my head is that someone hacking on the code might find that easier, when running yapf directly from the git repo.

[char101]

The metadata is only used to print the version and finding the user cache directory, both which are optional I think and shouldn't prevent running yapf as a normal package.

[eli-schwartz]

The metadata is only used to print the version and finding the user cache directory, both which are optional I think and shouldn't prevent running yapf as a normal package.

Whether it should or should not is academic: the actual code does this:

__version__ = metadata('yapf')['Version']

which will raise a fatal error if it cannot successfully parse the dist-info metadata and assign the resulting version number to a variable.

Whether the variable is ever used in a print statement is not really relevant.

[char101]

From another point of view, this is an exception that can be handled since it does not prevent the usage of yapf.

[eli-schwartz]

Yes, but also:

The reason originally given for using importlib_metadata was to avoid circular imports of some sort? Which seems odd but I never dug into the history.

I still am not sure I understand why it's necessary to retrieve this information using importlib.metadata; what, precisely, was the problem with sticking the version number inside __init__.py as the primary source of truth?

In theory, it may not be necessary to import any modules at all, whether from PyPI or the stdlib, or have any risk of it being unavailable.

[bwendling]

@bwendling should yapf support being run without any dependencies? Does that warrant any additional complexity in the application logic?

Having no dependencies would be nice, but probably difficult to achieve. So I'll say that having dependencies is okay, but we should try to limit them to only the most necessary ones. E.g. using metadata('yapf')['Version'] is a nice way to get version information, but if it's the only bit of code requiring that dependency, perhaps we should consider using another method to get the version.

[Spitfire1900]

I could remove importlib metadata() call dependency for getting the version info in yapf.__init__#L43 using setuptools dynamic metadata feature, statically specifying the value in __init__.py and having setuptools reference it.

But I alsu use it in driver.py#L210-L212 for providing arguments to platformdirs to store the Pickle cache.

[eli-schwartz]

What is the downside of having that code use

import yapf

...

def foo(): 
    x = user_cache_dir(version=yapf.__version__)

[Spitfire1900]

I've pushed up a changeset that uses static strings for the Author and Package name as well as pkgutil for the current version.

So the importlib-metadata dependency is removed.

[Spitfire1900]

What is the downside of having that code use

import yapf

...

def foo(): 
    x = user_cache_dir(version=yapf.__version__)

This triggers a circular import error.

[eli-schwartz]

This triggers a circular import error.

A common tactic people use in such cases is to have a file like: yapf/_version.py

So you could do:

# yapf/__init__.py
from yapf._version import __version__ as __version__

# yapf_third_party/_ylib2to3/pgen2/driver.py
import yapf._version

def foo():
    x = user_cache_dir(version=yapf._version.__version__)

Since yapf._version doesn't have a circular import, everything should be fine.

[Spitfire1900]

This triggers a circular import error.

A common tactic people use in such cases is to have a file like: yapf/_version.py

So you could do:

# yapf/__init__.py
from yapf._version import __version__ as __version__

# yapf_third_party/_ylib2to3/pgen2/driver.py
import yapf._version

def foo():
    x = user_cache_dir(version=yapf._version.__version__)

Since yapf._version doesn't have a circular import, everything should be fine.

I've migrated to this approach.

[bwendling]

Nice cleanup. Thank you!