tolgaatam / ColabTurtle

An HTML based Turtle implementation, in order to work in Google Colab
MIT License
55 stars 30 forks source link

position() method broken (not wrapped or implemented or exposed???) #4

Closed bdklahn closed 3 years ago

bdklahn commented 3 years ago
!pip3 install ColabTurtle
from ColabTurtle.Turtle import *
initializeTurtle()

forward(150)
left(90)
forward(150)
left(90)
forward(150)
left(90)
forward(150)
print(position())

(Turtle movement works fine)
. . .
---------------------------------------------------------------------------

TclError                                  Traceback (most recent call last)

<ipython-input-27-d50342cbc57b> in <module>()
     10 left(90)
     11 forward(150)
---> 12 print(position())

5 frames

/usr/lib/python3.7/turtle.py in position()

/usr/lib/python3.7/turtle.py in __init__(self, shape, undobuffersize, visible)
   3810                  visible=_CFG["visible"]):
   3811         if Turtle._screen is None:
-> 3812             Turtle._screen = Screen()
   3813         RawTurtle.__init__(self, Turtle._screen,
   3814                            shape=shape,

/usr/lib/python3.7/turtle.py in Screen()
   3660     else return the existing one."""
   3661     if Turtle._screen is None:
-> 3662         Turtle._screen = _Screen()
   3663     return Turtle._screen
   3664 

/usr/lib/python3.7/turtle.py in __init__(self)
   3676         # preserved (perhaps by passing it as an optional parameter)
   3677         if _Screen._root is None:
-> 3678             _Screen._root = self._root = _Root()
   3679             self._root.title(_Screen._title)
   3680             self._root.ondestroy(self._destroy)

/usr/lib/python3.7/turtle.py in __init__(self)
    432     """Root class for Screen based on Tkinter."""
    433     def __init__(self):
--> 434         TK.Tk.__init__(self)
    435 
    436     def setupcanvas(self, width, height, cwidth, cheight):

/usr/lib/python3.7/tkinter/__init__.py in __init__(self, screenName, baseName, className, useTk, sync, use)
   2021                 baseName = baseName + ext
   2022         interactive = 0
-> 2023         self.tk = _tkinter.create(screenName, baseName, className, interactive, wantobjects, useTk, sync, use)
   2024         if useTk:
   2025             self._loadtk()

TclError: no display name and no $DISPLAY environment variable

I was going to suggest my to my daughter's sixth grade CS teacher that he try Google Colab, instead of Trinket. But I guess it's not ready for prime time.

tolgaatam commented 3 years ago

Thank you very much for the issue that you created.

This library does not implement position(), it only has getx() and gety(). In your case, probably Python is calling standard Turtle position() function, as ColabTurtle does not implement it. And standard Turtle does not work in Colab, which is the reason I came up with this library :)

bdklahn commented 3 years ago

Thanks for the fast response! And thanks for writing this library! Yes, I was just looking through the code. I saw getx() and gety() and figured it wouldn't be too hard to implement pos(). I am not that familiar with the entire traditional turtle API, but I figured it might at least look good if it could run the standard official example. https://docs.python.org/3.7/library/turtle.html (I chose Python 3.7 docs version, because that is what Colab is apparently currently running)

I regularly use JupyterLab, and know a little about Colab. I saw my daughter's CS class was using Trinket, which only recently has Python 3 in the free version (and poor code practice examples). I knew there was better free software and services. So I thought I'd test to see if Colab could handle one of their early code examples, using Turtle. That's when I came across links pointing to this.

bdklahn commented 3 years ago

This might be a little overkill, but I wonder if it could be cool/useful to store information about the turtle's state in a Python dataclass. So it might be queried like. turtle_state.x_pos turtle_state.y_pos turtle_state.pen_color turtle_state.facing_direction turtle_state.is_pen_down etc.

I don't know . . . I kinda like to use such things to make it always clear and obvious to collaborators (including future me) what is there.

But I suppose maybe, instead, the "correct"/conventional way would be to store those as self attributes in a Turtle Class.

tolgaatam commented 3 years ago

When I look at the traditional API, there are many methods that I did not implement in this library. And I now see that I have used some function names differently than the original, as well.

I created this library for my university's introductory CS course, which I was the assistant of. We basically created our own lecture content and did not follow the Turtle documentation. Hence, we only needed a subset of the features, of course. Trying to comply with the traditional API would be too much of a hassle at that point (and also now πŸ˜€).

About your other comment, I implemented this library as a class initially (you can check the history of this repo for that implementation). But then, we remembered that Turtle would be one of the first things that the students learn about Python. For beginners, creating an instance from the class and calling the methods over the instance could be unnecessarily complex. That's why the library was then converted to be non-class.

At this point, my university (and maybe some other organizations too) is using this library. I would not wish to break the compatibility, nor populate the library with complex structures. However, few new getters etc. might be implemented, for sure! πŸ‘ I just need to remember my PyPi password πŸ˜†

bdklahn commented 3 years ago

Gotcha. I understand. I was wondering if this library, itself, was kind of an intro to programming.

Probably for general Colab end users, some library which "simply" (somehow) wraps the traditional Turtle library to replace the tkinter "DISPLAY" functionality/calls, might be more ideal for these cases where they might want to move their code (mainly just meant to get kids interested), to another platform.

Now that I think of it, though, it seems it could be in Jupyter or Colab's interest to make some way of more simply handling/adapting legacy tkinter-based code. Maybe there is, and I just haven't looked well enough. :-)

I am at a university also, but also help maintain production-grade software. So I am also familiar with university "in house" software.

Thanks for your time. At least this library is something which works. I could have fairly easily done what I wanted with "getx()" and "gety()". --and l like that/how it simply leverages html svg.

tolgaatam commented 3 years ago

At the time I was implementing the library (around 2018 I guess), there was no effort from Google to implement such a layer to connect tkinter with colab. That would be ideal, I agree. I am not aware whether they have implemented anything like that so far. At that time, using svg to draw on html was the only "hack" I could come up with. Thanks for finding it nice.

At another note, if you let me know which additional functions you might need (or propose), I can add them to this package as well. I guess I abondoned the project pretty much, by thinking nobody was interested in it apart from my university. But now I saw that there are some universities etc. giving homeworks on this library: this feels nice. And now I feel motivated to add some more functionality to the project :) Let me start by hearing your function recommendations.

bdklahn commented 3 years ago

Heh. I don't know. I just started really looking at it, and I probably had my daughter doing more advanced stuff than her teacher asked for (forward, right, penup, pendown, etc.).

I suppose it would be nice to have position() (and pos()) implemented.

I guess I was thinking if you are evoking a reminiscence of the "Turtle" module (by using that name), it would be nice if the implemented interface functions would match, where possible.

You could, in many cases, deal with not breaking your current user's expectations by simply assigning a second name for a function. e.g.

setheading = face

(assuming Python 3)

I was just looking at your "color()". It might not be too bad accommodating both your current users, and traditional Turtle users there, too. You could first check . . .

if  color in VALID_COLORS:
   # your code
elif:
    # color is a(n) (three-member tuple with) integer(s) between 0 and 255, etc.

. . .

else:
    # raise exception 

With things like that, and using default None parameter arguments to see what function "signature"/version a caller is expecting, etc., it might not be bad. If all the input arguments remain None after calling, then it must be the "getter" version the caller wants. If the first argument is a valid color input, but the second one stays "None", it is just the equivalent pencolor(. . .) setter, the caller wants. Hmmm . . . I haven't looked at the Turtle code, but it makes sense that such a function would return the color value(s) whether it is the setter or getter "version".

I don't know . . . I guess we used circle(). But the teacher didn't introduce it. That one is named/designed a little "weird" in that if you use a small number of steps, you see a polygon. e.g. a square, if steps=3 (I mean . . . it makes sense that you get a true circle when steps approaches infinity). Also, it might make more sense to have VALID_COLORS be a set vs. a tuple.

bdklahn commented 3 years ago

Yeah . . . In my mind, it was a totally appropriate decision by you to use html svg for this. The Turtle module (AFAIK) just ends up mostly drawing shapes and colors etc. From the start, Jupyter (ipynb) notebooks were designed with having html/markdown, etc., as their display. Svg tags fit nicely into this. Maybe an/the approach could be to fork the Turtle code, and create a small library of decorator functions. Then use those to wrap any low-level "private" functions used to handle/convert the ones using tkinter to using svg. I haven't looked at that code to see if it was well-separated to make it extensible.

bdklahn commented 3 years ago

Just for completeness . . . https://github.com/PythonTurtle/PythonTurtle/issues/159

Heh. If you want to take the Turtle baton . . . :-)

tolgaatam commented 3 years ago

I made most of your recommendations come true with the latest commit. You can have a look. Many new functions are added and the ones that were not accepting multiple signatures are updated to do so. Your recommendations are really making this project come alive. Thank you a lot! We should be really close to the traditional turtle API right now.

For the part that you mention using the original turtle code and writing a library with decorators, I think that would be a lot of work for me, as I am not super fluent in Python. If someone intends to work on such a solution, I guess it is more appropriate to keep it as another repo.

bdklahn commented 3 years ago

Wow! Awesome! I'll take a look at the code, when I get a chance. Yeah. It looks like it doesn't really make sense to use that wx module-based code, for this project (no decorators, etc.).

tolgaatam commented 3 years ago

@bdklahn Waiting for your complete analysis to release it as 2.1.0 to PyPi πŸšΆπŸ˜„

bdklahn commented 3 years ago

This seems to work for me, now.

tolgaatam commented 3 years ago

I'm glad it does. Closing the issue, then πŸ’―