Skip to content

turtle constructor incompatible with CPython: tuple expected at most 1 argument, got 2 #30

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
Closed
shulltronics opened this issue Jul 29, 2022 · 10 comments
Closed

turtle constructor incompatible with CPython: tuple expected at most 1 argument, got 2 #30

shulltronics opened this issue Jul 29, 2022 · 10 comments

Comments

@shulltronics
Copy link
Contributor

I'm working on adopting this library to use with CPython on Generic x86 computers. When I try to use this library I get the following error:

Traceback (most recent call last):
  File "/home/carstentb/Projects/intro-to-computing/L5/benzene.py", line 20, in <module>
    start = turtle.pos()
  File "/home/carstentb/Projects/forks/Adafruit_CircuitPython_turtle/adafruit_turtle.py", line 729, in pos
    return Vec2D(self._x - self._w // 2, self._h // 2 - self._y)
TypeError: tuple expected at most 1 argument, got 2

After some Googleing, I found that for child classes of Tuple, one should use the new constructor instead of init. I implemented this and got it working for CPython (see my fork here), but that breaks it on CircuitPython! When I test my changes on a Feather nRF52840 Express from CircuitPython 7.3.1, I get this error:

Traceback (most recent call last):                                                                                    
  File "code.py", line 32, in <module>                                                                                
  File "/lib/adafruit_turtle.py", line 736, in pos                                                                    
  File "/lib/adafruit_turtle.py", line 97, in __new__                                                                 
TypeError: function takes 1 positional arguments but 2 were given

Is this an incompatibility of CircuitPython and CPython? I would love to get my changes merged in here, but obviously that can't happen if my changes break this on the CircuitPython interpreter.
I would appreciate any comments or direction on how this might be resolved. Thanks! :)
@ladyada
Copy link
Member

ladyada commented Aug 1, 2022

@shulltronics does the original library work and then when you change things in your fork it doesnt? if so is there a PR with your changes?
@tekktrik wanna take a look?

@shulltronics
Copy link
Contributor Author

Thanks for getting around to this :) My original post could have been more clear.

Obviously the library works on CircuitPython devices. I want to use is on generic computers, and so the first thing I changed was to wrap the board import in a try block. This still works on CircuitPython too. See here for that simple fix.

However, once I made that change, I encountered the error mentioned above about "tuple expected at most 1 argument, got 2." After I did some research I tried changing the __init__ constructor of the Vec2D helper class to __new__. See my next commit here for that change.

This allowed me to use the library on my laptop, but unfortunately broke it on CircuitPython. I would love to make a PR so this library can work out-of-the-box on CPython with Blinka, but first I need to figure out how to sort out this constructor issue.

Thanks for your help, Adafruit!

@ladyada
Copy link
Member

ladyada commented Aug 1, 2022

@tekktrik @FoamyGuy - plz see if you have any suggestions

@FoamyGuy FoamyGuy mentioned this issue Aug 1, 2022
Open
@FoamyGuy
Copy link
Contributor

FoamyGuy commented Aug 1, 2022

There does appear to be a difference in behavior between CPython and CircuitPython with regards to subclassing tuple. I've created an issue to track that: (#6677).

The same difference is present in Micropython as well. I'm checking into whether this difference is intentional or not. If not, there may be some possibility to PR micropython to change it and that would eventually make it into circuitpython. If there is a reason for the difference then it may be kept as-is.

In any case the simplest solution in the short term for this turtle library is probably to refactor Vec2D so that it doesn't extend tuple.

As for your ultimate goal of running the turtle library on CPython. How are you intending to display it from CPython? using Blink_DisplayIO? or something different?

@tekktrik
Copy link
Member

tekktrik commented Aug 2, 2022 •
edited
Loading

I think I ran into an issue like that in the past, let me check the project I think it happened in when I get a chance sometime this week. Specifically, I noticed it for built-in types only, I believe, when subclassing from them, but I'll double-check.

@tekktrik
Copy link
Member

tekktrik commented Aug 2, 2022

I checked, I actually think it may be related to any class defined in C - I believe a similar instance happened when I tried to subclass displayio.Group.

@shulltronics
Copy link
Contributor Author

Thank you everyone for you interest and help with this. I will be following 6677 to see the discussion there, and also keeping up here.

@FoamyGuy yes I used Adafruit_Blinka_Displayio to show the graphics in a PygameDisplay. (Remember helping me before with this?? :)

I'm going on a two week vacation starting Thursday so this might sit a bit stagnant until then. But I will revisit and try to get a working solution then. Cheers!

@tekktrik tekktrik removed their assignment Aug 17, 2022
@shulltronics
Copy link
Contributor Author

Hi all,

I believe I have fixed this issue. The solution, as @FoamyGuy previously mentioned, was to remove the tuple subclass from the Vec2D class -- which was amazingly simple, but I just hadn't taken the time to revisit this until now (I have time off work thanks to hurricane Ian!)

Basically, in addition to removing the subclass, I changed __init__(self, x, y) to __new__(cls, x, y), and then return a tuple directly from the constructor, instead of calling the super initializer. I'm not sure this is actually much different from doing the subclass method, because the instance seems to literally be a tuple, and the indexing in the subsequent methods works fine.

See here for my code. I have tested this on my linux machine, and on a Feather nRF52840 with CircuitPython 7.3.1, so I believe it works properly. I'm going to open a PR for this fix. Let me know if you all have any questions. Thanks!

@shulltronics
Copy link
Contributor Author

shulltronics commented Sep 28, 2022 •
edited
Loading

#31 Pull Request open :)

@FoamyGuy
Copy link
Contributor

FoamyGuy commented Oct 3, 2022

resolved by #31

@FoamyGuy FoamyGuy closed this as completed Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ladyada @FoamyGuy @shulltronics @tekktrik