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

Require Python 3.7+, fix test failures, test against Py 3.10 and 3.11, README/RELEASE update #19

Merged
merged 12 commits into from
Nov 1, 2022

Conversation

consideRatio
Copy link
Member

@consideRatio consideRatio commented Nov 1, 2022

Based on #18 to avoid merge conflicts. Excluding that #18, this PR involves a flurry of minor fixes.

  • ci: fix failure to test old python versions
  • Transition to hatchling/pyproject.toml from setuptools/setup.py
  • Let module include __version__
  • Fix typo in comment
  • Add a basic description to the README
    Closes Provide a non-technical introduction paragraph in the readme #15
  • Use relative module imports within the module
    Closes Fix CI test failures #12
  • ci: reduce verbosity of pytest
  • Update RELEASE.md and rely on tbump
  • Drop support for Python 3.6
  • Adopt python 3.7 text=True for subprocess.Popen
  • docs: increase development status beyond beta
  • ci: resolve intermittent test failures by sleeping longer
    Properly closes Fix CI test failures #12

DRAFT

Currently working to fix test failing intermittently

@consideRatio consideRatio marked this pull request as draft November 1, 2022 16:21
@consideRatio consideRatio force-pushed the pr/small-fixes-and-readme branch 2 times, most recently from e5122fd to f1aa2a2 Compare November 1, 2022 16:43
@consideRatio consideRatio changed the title Minor refactoring and basic description to README Minor refactoring, basic description to README, fix test failures and verify 3.10 and 3.11 function Nov 1, 2022
@consideRatio consideRatio marked this pull request as ready for review November 1, 2022 16:46
@consideRatio consideRatio marked this pull request as draft November 1, 2022 17:05
@consideRatio consideRatio force-pushed the pr/small-fixes-and-readme branch 4 times, most recently from 5d8ff67 to fc4db44 Compare November 1, 2022 17:48
@consideRatio consideRatio changed the title Minor refactoring, basic description to README, fix test failures and verify 3.10 and 3.11 function Require Python 3.7+, README update, fix test failures, test against Py 3.10 and 3.11 Nov 1, 2022
@consideRatio consideRatio marked this pull request as ready for review November 1, 2022 17:53
@consideRatio consideRatio changed the title Require Python 3.7+, README update, fix test failures, test against Py 3.10 and 3.11 Require Python 3.7+, fix test failures, test against Py 3.10 and 3.11, README/RELEASE update Nov 1, 2022
@yuvipanda
Copy link
Collaborator

@consideRatio do we use hatchling in other Jupyter projects already? Also I merged #18, but github doesn't seem to acknowledge that here in showiong me diffs :(

@consideRatio
Copy link
Member Author

consideRatio commented Nov 1, 2022

Rebased! Thanks @yuvipanda!!!

I saw @minrk start to use it in https://github.com/minrk/jupyter-keepalive/blob/main/pyproject.toml so that is my inspiration towards this.

@consideRatio
Copy link
Member Author

I'm not sure, do you need that defined at all? Would it work to remove it entirely and have a pyproject.toml without hatchling defined as a build system?

Copy link
Collaborator

@yuvipanda yuvipanda left a comment

Choose a reason for hiding this comment

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

@consideRatio so I'm totally lost on pyproject.toml in general, and haven't been keeping up :) My only requirement is that 'editable installs' (pip install -e .) work. If it does for hatchling, happy to give it a shot here :) Do you know if it does? I can't seem to find info...

@consideRatio
Copy link
Member Author

consideRatio commented Nov 1, 2022

image

I'm quite lost as well, but I think this is fine! I'm happy to avoid having setup.py, setup.cfg, pyproject.toml all side by side. If adding hatchling to pyproject.toml makes us able to remove the setup.py thats nice.

Regarding editable builds

I think can't use Python 3.5 and remove setup.py going for only pyproject.toml + a declared build-system, but you can if you have python 3.6+! So, this pattern is something we can adopt wider as well I think.

Atm, you can use pip install -e . on this project and it will work!

@yuvipanda yuvipanda merged commit 02f6db6 into jupyterhub:main Nov 1, 2022
@yuvipanda
Copy link
Collaborator

Thanks, @consideRatio! If pip install -e . works I'm happy to try :)

@consideRatio
Copy link
Member Author

Wiee thanks for your quick review of this @yuvipanda, greatly appreciated!!! ❤️ 🎉

@minrk
Copy link
Member

minrk commented Nov 2, 2022

I've been exploring hatchling, which is kinda the PyPA successor to flit, but with hooks for optional build steps. It's being adopted in the JupyterLab/jupyter-releaser projects, so there's a plugin to build jupyterlab extensions, so we don't have to cargo-cult our npm build steps around everywhere in setup.py. That's why I picked it for jupyter-keepalive. I probably would have stuck with setuptools in keepalive were it not for the lab extension.

If you have a simple Python-only package like this one, there isn't really a difference between using setuptools and hatchling with pyproject.toml. You don't need setup.py with either one, but if you have setup.py with setuptools, it's the two-line from setup.py import setup; setup() since you can put all your metadata in pyproject.toml's [project] section instead of args to setup.

pip install -e does work for pyproject.toml installs, thanks to pep 660.

As I understand hatchling more, I think we can make the jupyterhub build/install much simpler than the complicated steps we have now.

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.

Provide a non-technical introduction paragraph in the readme Fix CI test failures
3 participants