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

Adopt NEP 29 #63

Merged
merged 10 commits into from
Oct 26, 2020
Merged

Adopt NEP 29 #63

merged 10 commits into from
Oct 26, 2020

Conversation

jamesmyatt
Copy link
Contributor

@jamesmyatt jamesmyatt commented Oct 22, 2020

  • Removes support for Python 3.6 and earlier, in line with NEP 29
  • Adds CI for Python 3.8 and 3.9

Closes #62, #61

- python: 3.7
- python: 3.6
- python: 3.5
- python: 2.7
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Regarding Python 2.7, any particular reason why it was removed like incompatibility with the PR? Personally, I removed Python 2.7 support from all my main projects, but for this one, I think it's quite useful. I.e., I use watermark for teaching (have it on top of HW code that students submit), and I thought having Python 2.7 support would make it useful to catch the use of Python 2.7 (to explain certain bugs etc.)

Copy link
Contributor Author

@jamesmyatt jamesmyatt Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just about new features. You will still be able to install existing versions of watermark on Python 2.7: that's the point of the python_requires property.

But NEP 29 is adopted by all major Scientific Python projects. There's also https://python3statement.org/. So if you want to use the latest Scikit-learn or Numpy or Pandas or IPython, then you will need Python 3.7+ anyway, so there's no point having the maintenance burden of Python 2.7 for future releases.

Copy link
Owner

@rasbt rasbt Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But NEP 29 is adopted by all major Scientific Python projects. There's also https://python3statement.org/. So if you want to use the latest Scikit-learn or Numpy or Pandas or IPython, then you will need Python 3.7+ anyway, so there's no point having the maintenance burden of Python 2.7 for future releases.

Totally agree and I did that for all my scientific computing related projects. For watermark though, I do think one of the usecases is to diagnose/see what code someone is using or has used, which could be legacy Python 2.7 and old libraries.

For me, one use case is basically when students email me that the HW template code doesn't run for some reason (new programmers usually tend to forget sending error messages). I then usually ask them to send me the watermark outputs, and often it's because they haven't set their environment correctly and Jupyter Notebook is using a Python 2.7 kernel and/or they are using some old NumPy versions.

We may have to drop Python 2.7 support one day, but since Python 2.7 is still the default on many systems, I think it's useful to support it, to see whether someone was running code on Python 2.7 (and then recommend this person to switch to Python >= 3.7). So, I would suggest adding Python 2.7 back to the travis.yml.

Copy link
Contributor Author

@jamesmyatt jamesmyatt Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But someone using Python 2.7 will always be able to get watermark 2.0.2 via pip, so the current functionality will always work, and that will tell you what version of Python, etc they are using.

And if they try to use any hypothetical new future functionality, then the fact it doesn't work will show they're on the wrong version of Python. Also they will be stuck on IPython 5.8 (Jul 2018), numpy 1.16.5 (Aug 2019), pandas 0.24.2 (Mar 2019), scikit-learn 0.20.4 (July 2019), etc.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with your points. I was just thinking it might be good to keep 2.7 in the travis CI to see at what point in future 2.7 will not work anymore. I am not suggesting we should actively keep supporting 2.7; it might just be useful to know whether it fails or not.

Copy link
Contributor Author

@jamesmyatt jamesmyatt Oct 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The python_requires setting will stop it installing. Although there's probably an option to ignore that.

update: The pip option is --ignore-requires-python

@rasbt
Copy link
Owner

rasbt commented Oct 26, 2020

Awesome, thanks a lot. Thx also for cleaning up the init.py and conda stuff. Really appreciate it.

@rasbt rasbt merged commit ff4ee37 into rasbt:master Oct 26, 2020
@jamesmyatt
Copy link
Contributor Author

No problem. Can you label this with hacktoberfest-accepted, please?

@jamesmyatt jamesmyatt deleted the feature/nep-29 branch October 27, 2020 13:29
@jamesmyatt jamesmyatt mentioned this pull request Oct 27, 2020
@rasbt
Copy link
Owner

rasbt commented Oct 27, 2020

sure. done.

@Jhsmit Jhsmit mentioned this pull request Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adopt NEP 29?
2 participants