Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of pkg_resources for package versions #41

Closed
jamesmyatt opened this issue Oct 12, 2018 · 11 comments
Closed

Use of pkg_resources for package versions #41

jamesmyatt opened this issue Oct 12, 2018 · 11 comments

Comments

@jamesmyatt
Copy link
Contributor

jamesmyatt commented Oct 12, 2018

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/.

@jamesmyatt
Copy link
Contributor Author

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
Copy link
Owner

rasbt commented Oct 12, 2018

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

@jamesmyatt
Copy link
Contributor Author

jamesmyatt commented Oct 13, 2018

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
Copy link
Owner

rasbt commented Oct 13, 2018

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.

@jamesmyatt
Copy link
Contributor Author

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
Copy link
Owner

rasbt commented Oct 14, 2018

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?

@jamesmyatt
Copy link
Contributor Author

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
Copy link
Owner

rasbt commented Oct 15, 2018

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?

@jamesmyatt
Copy link
Contributor Author

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

@rasbt
Copy link
Owner

rasbt commented Oct 4, 2019

Should be addressed now via #54 (using pkg_resources)

@rasbt rasbt closed this as completed Oct 4, 2019
@jamesmyatt
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants