rasbt/watermark

Use of pkg_resources for package versions

jamesmyatt opened this issue · 11 comments

Is there a reason why _get_packages doesn't use pkg_resources to get the package version numbers, like pip does?

The version attributes inside the imported modules are not guaranteed to exist or to match the version that pip (or conda) understand. pkg_resources is the way to do this. Otherwise you risk one or more of the following recommended approaches not working: https://packaging.python.org/guides/single-sourcing-package-version/.

I've looked at #15, but I don't understand the issue. Surely the package names and numbers reported via pip are the ones that you want to report, rather than the value of some internal attribute.

rasbt commented

Is there a reason why _get_packages doesn't use pkg_resources to get the package version numbers, like pip does?

Honestly has been so long that I don't remember why I chose one over the other, but I think pkg_resources does not work with older versions of Python setuptools?

I think it's useful to support these Python versions for people who deliberately have to use an old python version on a webserver or so. On the other hand, this is a IPython magic command, so there may not be an intersection of people who have IPython installed but yet have such an old setuptools version.

So I agree with you changing to pkg_resources makes sense

I'm happy to make these changes when I have time, but it will be a major version since it changes the meaning of the -p flag (from modules to packages).

It is worth dropping guaranteed support for Python 2 at the same time? i.e. for version 2.0. IPython hasn't supported it 2 major versions ago. https://python3statement.org/. In fact, IPython 7.0+ only supports Python 3.5 and above.

rasbt commented

It is worth dropping guaranteed support for Python 2 at the same time?

That's a tough question. Making features like this easier to implement is exactly the reason why I think people should "slowly" start about dropping Py 2 support if they haven't done so already.

However, pip install ipython it will still install IPython no matter which Python version you have, and one of the use cases of this package is to find out more about the environment that a user used to run/execute particular code (when my students have questions regarding hw submissions, it's super handy to tell them to execute %watermark ... in the notebook and report back the output of this.

So, I am afraid dropping Py2 makes this less useful, because people who are voluntarily or involuntarily using Python 2.7 but then unable to install watermark for a reason unknown to them (thinking of beginners), and then it will just be another hurdle to them to find out why they can't install it etc..

Anyways, usually, I am in favor of dropping Py 2 support, but maybe we shouldn't do that just yet. I do think though a (deprecation) warning should be displayed if a person runs watermark in IPython on Python 2.7 as a start.

If the change was made in v2.0 then v1.x would still install on Python 2.7 just as IPython 5.x does

rasbt commented

That sounds good to me then. Have never done that before -- you are basically saying we can specify the IPython/Python dependency of the 2.0 version somewhere in the setup.py file so that people on Python 2.7 get the correct (compatible) 1.7 version when they run pip install watermark?

Started work on this here: https://github.com/jamesmyatt/watermark/tree/v2-pre

I also noticed that some of the methods already use the Python 3 print function without importing from future:

print('{:<10} {}'.format(val.__name__, val.__version__))

rasbt commented

Good point. That's a contributed function, personally, I don't use format, just didn't think about it when I merged it. But wouldn't it also still work in Python 2.7?

I think that str.format was backported from Python 3.0 to Python 2.6

rasbt commented

Should be addressed now via #54 (using pkg_resources)

Thanks for doing this 👍
I've been on parental leave, so I've only just noticed 😆