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

Add typing / drop Python < 3.8 support #30

Merged
merged 7 commits into from
Jan 29, 2024

Conversation

nhairs
Copy link
Contributor

@nhairs nhairs commented Jan 24, 2024

This PR adds Python 3.8+ compatible type annotations (dropping support for <3.8 at the same time).

Fixes: #27

At a high level the following changes have been made:

  • Update code to use new features available from 3.8 (e.g. f"{foo=} notation).
  • Type annotations
    • note: Because we use from __future__ import annotations we can use "new style" annotations (e.g. str | list[int] | None instead of Optional[Union[str, List[int]]]) which are normally only available in higher versions of python. However it does mean that these annotations cannot be resolved from their string representation (all type annotations are stored as strings which is why this works) into their concrete classes unless you are running a supported version of Python.
  • Completely move to pyrproject.toml
  • Move all development dependencies to pyproject.toml, make make .venv more portable
    • virtualenv isn't always available so use -m venv .venv which is just as effective, and not all systems have python(2) installed or python3-is-python so explicitly use python3
  • Add some new badges to README

Test Plan

  1. Ensure that make ready-pr runs without error

foo.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@nhairs nhairs marked this pull request as ready for review January 26, 2024 04:05
@toumorokoshi
Copy link
Owner

to fix the tests, I think you have to remove setup.cfg from this line: https://github.com/toumorokoshi/deepmerge/blob/master/Makefile#L4

Copy link
Owner

@toumorokoshi toumorokoshi left a comment

Choose a reason for hiding this comment

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

Wow, awesome! thank you so much - this is fantastic.

LGTM overall, a couple smaller nits.

Once the tests pass happy to merge it in!

deepmerge/strategy/core.py Outdated Show resolved Hide resolved
deepmerge/strategy/core.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@nhairs nhairs changed the title Add typing / drop Python < 3.7 support Add typing / drop Python < 3.8 support Jan 26, 2024
@nhairs
Copy link
Contributor Author

nhairs commented Jan 26, 2024

Made a few other changes, including the Makefile. Now waiting on CI job being unblocked :)

There is a chance this will break a lot of code so consider doing a rc/a release first (if you do I can test it in my own projects).

Also you'll probably want to avoid a merge commit because I have complete disregard for git history within PRs.

@toumorokoshi
Copy link
Owner

Made a few other changes, including the Makefile. Now waiting on CI job being unblocked :)

There is a chance this will break a lot of code so consider doing a rc/a release first (if you do I can test it in my own projects).

Also you'll probably want to avoid a merge commit because I have complete disregard for git history within PRs.

Thanks! I haven't had a chance to take another pass, but I did unblock CI. I'll see if I can just give your PRs permission to run without approval.

ack on RC - I was thinking the same myself, we'll ship a beta, maybe a major version too just to mark the lack of support for py2.

No worries on Git history - I'll squash at the end ;)

@nhairs
Copy link
Contributor Author

nhairs commented Jan 27, 2024

I'll see if I can just give your PRs permission to run without approval.

Based on other repositories I've created PRs for I believe this is possible; though I'm not sure what the setting is as I don't use GHA myself.

@toumorokoshi toumorokoshi merged commit 3bb38cb into toumorokoshi:master Jan 29, 2024
5 checks passed
@toumorokoshi
Copy link
Owner

awesome, thanks! will get a beta version out soon.

@toumorokoshi
Copy link
Owner

@nhairs beta is out: https://pypi.org/project/deepmerge/2.0b0/. Would you mind testing it out?

@nhairs
Copy link
Contributor Author

nhairs commented Jan 30, 2024

Tested it on a couple of projects, /seems/ okay 🙂

I didn't get any lint errors (which is kind of expected if I'm calling things correctly). Didn't get any errors. Did have to drop py37 support in one of my projects 😅

@nhairs nhairs deleted the typing branch January 31, 2024 02:29
@nhairs
Copy link
Contributor Author

nhairs commented Aug 30, 2024

Hi @toumorokoshi,

It's been 6 months since the 2.0 beta release. Would you be able to upload a full release to PyPI?

@toumorokoshi
Copy link
Owner

done! thanks for the reminder: https://pypi.org/project/deepmerge/2.0/

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

Successfully merging this pull request may close these issues.

Add Type Hints
2 participants