Use importlib.metadata if present

[jayvdb]

Closes #17

[sinoroc]

Thanks for the pull request!

I believe the CI setup may be flawed, so that such a change would never pass the checks. I would need to have a closer look to be sure of that, but that's my impression. I will let you know, once I figure this out and if a change of the CI setup is needed.

On the other hand (unrelated to the CI setup flaw), there is a remaining issue in the declaration of the dependencies. Indeed for Python versions that do have importlib.resources, we don't need to install importlib-resources. So an adjustment is needed in setup.cfg.

[jayvdb]

How about I change the CI so that mypy is only used on py38?

And yes, I can adjust setup.cfg to not install the unnecessary dependency.

[sinoroc]

I can adjust setup.cfg to not install the unnecessary dependency.

Yes, please.

How about I change the CI so that mypy is only used on py38?

I'd rather do the CI changes myself. Thanks. There are more things I want to change about it anyway. But I will be upfront: I don't know when, meaning your pull request might stay pending for a while. The importlib-metadata change is not urgent anyway, so that shouldn't be a problem. Or do you see any reason to speed things up?

[sinoroc]

Another thing, I don't want the commit messages to be too tied to GitHub (for example I also host the same code on GitLab). In the commit message we need to be more explicit about what these magic numbers actually refer to, so please transform:

Closes #17 

into:

GitHub: closes #17 

I should add this to the contributing guidelines.

[jayvdb]

fwiw, my commit message has an explicit URL, not just a #xx precisely so it solves that problem. A practise enforced by @coala which also has gitlab and github repos. But I am happy to follow whatever convention you wish.
Please let me know when the CI is fixed.

The importlib-metadata change is not urgent anyway,

It makes your plugin incompatible with distros which do not provide packages for importlib-metadata and importlib-resources (and dataclasses and other backports), so it is impossible to import them without installing from pip into system packages, never a good idea.

(However sed in my .spec fixes this, so not urgent on my account)

[sinoroc]

fwiw, my commit message has an explicit URL, not just a #xx precisely so it solves that problem. A practise enforced by @coala which also has gitlab and github repos. But I am happy to follow whatever convention you wish.

I see. Well actually I don't see it, because GitHub somehow does not show the URL, it only shows the #XX, which I don't like (or maybe I don't know where to look, which is not impossible). I should checkout the commit to see it, I guess.

Please let me know when the CI is fixed.

Yes

The importlib-metadata change is not urgent anyway,

It makes your plugin incompatible with distros which do not provide packages for importlib-metadata and importlib-resources (and dataclasses and other backports), so it is impossible to import them without installing from pip into system packages, never a good idea.

I understand. Maybe you will find this workaround useful (Tox's requires setting):

[tox]
requires =
    tox-poetry-dev-dependencies

(However sed in my .spec fixes this, so not urgent on my account)

Good :)

[sinoroc]

So the CI, is "fixed".

Honestly if the workaround with requires in tox.ini is good enough for your use cases, then I'd rather not merge this change (at least not for now). I never liked these try-import-except-import things, I know they are a very common pattern, but I'd rather avoid them if I can.

I am open to reconsider, if it's a strict blocking issue for anyone.

[jayvdb]

That workaround neither works, nor is it relevant.
requires doesnt install for tox plugins, because tox-plugins run outside of the venvs.

The backport you are depending on is not supposed to be used on versions of Python which have a better stdlib version of the library. These backports degrade over time, as maintaining it is duplicate effort.

[sinoroc]

I am confused...

That workaround neither works, nor is it relevant.
requires doesnt install for tox plugins, because tox-plugins run outside of the venvs.

Tox and its plugins absolutely should be installed and run outside of the test environments. What am I missing here?

The backport you are depending on is not supposed to be used on versions of Python which have a better stdlib version of the library. These backports degrade over time, as maintaining it is duplicate effort.

I see your point. But in that particular case, as far as I know importlib-metadata is still being actively maintained. It looks to me like it even got some fairly recent improvements regarding speed, consistency, reliability. It is not clear to me if these improvements have already been released in Python's standard library version (my impression is that Python 3.8 is more or less at the level of importlib-metadata v1.5.0).

I'm trying to get a clearer idea of the pros and cons here: https://gitlab.com/python-devs/importlib_metadata/-/issues/130

[jayvdb]

requires installs items into the venvs (including build isolation venvs). It doesnt install items outside the venvs, so it is irrelevant to tox plugins which exist in the same environment as tox. tox does use requires for tox plugins, but only for the version specification, but if the requires of plugins are missing, tox cant help install them. This isnt well explained by the tox docs, but you can see this more clearly in the relevant PR when it was added.

[sinoroc]

Maybe what I am talking about was only introduced in Tox 3.8, with the auto-provision feature, while requires was introduced in 3.2 (the documentation and the changelog are a bit unclear about that). What version of Tox are you using, targeting?

[jayvdb]

I am talking about talking about tox master internals.

anyway, CI is indeed fixed. Nice. I'll get this PR updated to not depend on the backport when it isnt needed.

[sinoroc]

but if the requires of plugins are missing, tox cant help install them

As far as I know, since Tox 3.8 it does:

requires [...] Specify python packages that need to exist alongside the tox installation for the tox build to be able to start [...] Use this to specify plugin requirements [...] If these dependencies are not specified tox will create provision_tox_env environment so that they are satisfied and delegate all calls to that.

-- https://tox.readthedocs.io/en/latest/config.html#conf-requires

Also:

• https://tox.readthedocs.io/en/latest/example/basic.html#auto-provision

[sinoroc]

@jayvdb

I am still not convinced such a change is worthwhile. In my mind it is not a blocking issue (especially with the auto provisioning feature in Tox 3.8+). If the dependency on importlib-metadata really is problematic, then I think I'd rather remove the __version__ definition completely. This __version__ is nice to have, but I doubt it is really used by anyone. What do you think? Would that satisfy your use cases?