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.

Sign up for GitHub

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

Jump to bottom

Add functools.total_ordering #818

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add functools.total_ordering #818

wants to merge 1 commit into from

Conversation

winni2k
Copy link

@winni2k winni2k commented Mar 4, 2024 •
edited
Loading

As part of my journey to port dataclasses to micropython-lib/python-stdlib (see https://github.com/orgs/micropython/discussions/13741#discussioncomment-8579351), I needed to port functools.total_ordering first.

I have tried to follow all the contribution guidelines, but I have not tried to optimize code size as I am completely new to micropython and would not know where to start.

This code (and test code) is copied from CPython v3.7.17 in its entirety. The PSF license appears to allow for this, but please do correct me if I am wrong.

@winni2k
Copy link
Author

winni2k commented Mar 8, 2024

Any chance you could review this PR @mattytrentini ?

@mattytrentini
Copy link
Contributor

Any chance you could review this PR @mattytrentini ?

Sorry, been trying to find time to do so!

Perhaps consider adding a README.md explaining how to run the tests? Ideally in a container but even if it's just a description it would be helpful (I mean, it looks like straightforward unittest code but it should be clarified). Eventually we'd like to create Github Actions to run them continuously so if we have clear instructions it'll make that task easier.

It's also important to spell out that this code is licensed under the Python license. See the root LICENSE file that mentions MIT "unless explicitly stated otherwise". I think an entry in the README will suffice.

Otherwise, the code looks good! I'm curious about the failing unit test but I'll have to look into that another time...

(PS thanks for the contribution!)

@winni2k winni2k force-pushed the functools branch 3 times, most recently from 78ccecd to 13285a4 Compare March 19, 2024 14:33
@winni2k
functools: Add functools.total_ordering.
61efc92 
This commit is the result of copying the total_ordering code and tests
over from CPython v3.7.17.
One test is disabled because it expects builtin objects to have
attributes (__lt__, __gt__, etc.).
Another test for compatibility with pickle is also disabled because
pickle compatibility is currently broken.
Bumped package version to 0.0.8.

The functools code in CPython has the following credits:

Written by Nick Coghlan <ncoghlan at gmail.com>,
Raymond Hettinger <python at rcn.com>,
and Łukasz Langa <lukasz at langa.pl>.
  Copyright (C) 2006-2013 Python Software Foundation.
See C source code for _functools credits/copyright

This work was donated by W Winfried Kretzschmar.

Signed-off-by: W Winfried Kretzschmar <[email protected]>
@winni2k
Copy link
Author

winni2k commented Mar 19, 2024

Have a look now.

  • moved the PSF licensed code into a separate file, which required creating a package for functools.
  • added instructions for running tests using Docker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@winni2k @mattytrentini