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

feat: add queue support #50

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

imnotjames
Copy link
Contributor

@imnotjames imnotjames commented Oct 30, 2023

add implementation for asyncio.Queue that matches python's asyncio signature for Queue

Fixes #6

@imnotjames
Copy link
Contributor Author

I don't see an easy way to add tests - they don't seem to exist for this repo. Happy to change as needed.

@imnotjames
Copy link
Contributor Author

Err, ok, it's missing copyright info in the file. Should I use the same one from the other where it references the micropython license?

@imnotjames imnotjames marked this pull request as ready for review October 30, 2023 02:38
@dhalbert
Copy link
Contributor

You can use pre-commit locally to fix up various things: https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/check-your-code

@dhalbert
Copy link
Contributor

Err, ok, it's missing copyright info in the file. Should I use the same one from the other where it references the micropython license?

Yes, you can copy the original copyright.

@imnotjames
Copy link
Contributor Author

Err, ok, it's missing copyright info in the file. Should I use the same one from the other where it references the micropython license?

Yes, you can copy the original copyright.

Got it, added!

asyncio/queue.py Outdated Show resolved Hide resolved
@@ -0,0 +1,145 @@
# SPDX-FileCopyrightText: 2019-2020 Damien P. George
Copy link

@reapzor reapzor Nov 1, 2023

Choose a reason for hiding this comment

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

SPDX-FileCopyrightText: Copyright (c) 2023 Your Name Here / Handle? for Adafruit Industries ? What's the right approach here? it seems wrong copying this copyright verbatim

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per @dhalbert I copied the original copyright

Copy link
Contributor

Choose a reason for hiding this comment

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

"for Adafruit Industries" is when someone got paid to do the work.

@imnotjames imnotjames force-pushed the support-queue branch 2 times, most recently from f23de4e to ea4f3d1 Compare November 14, 2023 00:01
@imnotjames
Copy link
Contributor Author

Added some tests - once #58 is merged in these confirm the Queue behavior

@FoamyGuy
Copy link
Contributor

FoamyGuy commented Dec 18, 2023

@imnotjames this was passing the current actions at commit f94cfb3, but no longer is.

I'm not sure of the current state of the discussion or issues that you noted in #58 around testing but perhaps we can raise that topic during a weekly meeting (next one is scheduled for Jan 2nd) if you want to try to move the discussion forward and get more folks involved.

In the meantime if you're interested / willing if you can get this branch back to the state it was at for f94cfb3 so that it's back to passing actions I can have a look over it and try to get it reviewed and merged without being blocked by the testing.

@imnotjames
Copy link
Contributor Author

@imnotjames this was passing the current actions at commit f94cfb3, but no longer is.

I'm not sure of the current state of the discussion or issues that you noted in #58 around testing but perhaps we can raise that topic during a weekly meeting (next one is scheduled for Jan 2nd) if you want to try to move the discussion forward and get more folks involved.

In the meantime if you're interested / willing if you can get this branch back to the state it was at for f94cfb3 so that it's back to passing actions I can have a look over it and try to get it reviewed and merged without being blocked by the testing.

The issue is that there are tests which pytest is trying to run but it can't because they aren't pytest.

I can drop these tests for now and eventually merge them into the circuitpython core; It just means that the queue is "untested".

@imnotjames
Copy link
Contributor Author

I'm going to drop that commit and force push to remove it from the history of this PR.

I'll save them as a gist: https://gist.github.com/imnotjames/d281bc6a6a7cc6fc964e30e080560ed8

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 Queue and other asyncio features from MicroPython unofficial version of asyncio
4 participants