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

Removing g from the list of py5 "reserved words" breaks things #317

Closed
hx2A opened this issue Jul 15, 2023 · 10 comments · Fixed by #321
Closed

Removing g from the list of py5 "reserved words" breaks things #317

hx2A opened this issue Jul 15, 2023 · 10 comments · Fixed by #321

Comments

@hx2A
Copy link
Collaborator

hx2A commented Jul 15, 2023

I can define a variable named g but I can't actually use it. Simple example:

g = 42
x = g + 10

result:

py5 encountered an error in your code:


    1    g = 42
--> 2    x = g + 10
    3    
    ..................................................
     g = 42
    ..................................................

TypeError: 'int' object is not callable

I don't know why I didn't find this problem before. I guess when I tested it I only tested I could create a variable named g but didn't use it for anything.

I agree with @villares 's logic of allowing g as a variable name because single character reserved words are problematic. However, this configuration isn't going to work.

The solution is to rename g to something else, such as graphics. The new name will become a reserved word. @villares, are you ok with renaming g to graphics and making it a reserved word? Any other name suggestions?

@villares
Copy link
Contributor

Ideally, if you could rename g to graphics but leave a garden variety global variable g that points to the graphics named object that would get the best of both worlds wouldn't it? I mean one could override g with attribution and still have it for retro-compatibiity. Too much complexity?

@hx2A
Copy link
Collaborator Author

hx2A commented Jul 15, 2023

Ideally, if you could rename g to graphics but leave a garden variety global variable g that points to the graphics named object that would get the best of both worlds wouldn't it? I mean one could override g with attribution and still have it for retro-compatibiity. Too much complexity?

That's a nice idea for compatibility but it won't work. The same issue above would still occur.

Here's a more compatibility-friendly alternative: add graphics in addition to g, both referring to the same object, working the same way and both reserved, except for a deprecation warning whenever someone uses g. The warning would instruct them to use graphics instead. Then remove g completely in a future release, allowing the use of g as a variable name.

I'm fine with either way, but note that py5 is still labeled as "alpha" software, and these kinds of changes are allowed. I forget exactly what all the rules are for "alpha", "beta", etc. but I remember this part. :) I have been thinking about moving py5 from alpha to beta though...getting closer to the point where it makes sense to do that. The above alternative is a better practice for non-alpha libraries.

Edit: after thinking about it for a bit, this deprecation warning alternative is the better way to go. I am sure this change will break someone's code and I don't want frustrated people wasting time debugging what will seem to them to be a confusing problem.

@villares
Copy link
Contributor

I'll go with your decision, but I don't understand why we can't have something like g = py5.graphics added as a header somewhere so that g is not tracked at all (being able to be shadowed).

I suppose mine is the mindset of the small script/sketch builder who can hack "ad hoc" names in the global namespace, not the mindset of the library builder.

@hx2A
Copy link
Collaborator Author

hx2A commented Jul 15, 2023

I'll go with your decision, but I don't understand why we can't have something like g = py5.graphics added as a header somewhere so that g is not tracked at all (being able to be shadowed).

Hmmm, let me look into that. I just typed out an explanation about why it can't be like that because of how imported mode works, but then I realized it might work after all. Let me do some investigation and see.

@villares
Copy link
Contributor

After thinking about the new name, maybe graphics is too generic and maybe more likely to be used by people as a variable name, should we choose something a bit less concise? Here some ideas, I'm not really attached to any:
g_object, graphics_buffer, graphics_context, sketch_graphics. If you feel making it different would be too much, in the end of the day, graphics would certainly be OK.

@hx2A
Copy link
Collaborator Author

hx2A commented Jul 16, 2023

After thinking about the new name, maybe graphics is too generic and maybe more likely to be used by people as a variable name, should we choose something a bit less concise?

Well, py5 already has circle, box, stroke, line, and color, so don't think graphics would be unusual. Your alternative suggestions are also good ideas. I would add primary_graphics or main_graphics to the list.

@hx2A
Copy link
Collaborator Author

hx2A commented Jul 16, 2023

I'll go with your decision, but I don't understand why we can't have something like g = py5.graphics added as a header somewhere so that g is not tracked at all (being able to be shadowed).

Hmmm, let me look into that. I just typed out an explanation about why it can't be like that because of how imported mode works, but then I realized it might work after all. Let me do some investigation and see.

OK, I sat down to figure this out and it seems I would need to add too many hacks to the code for this to work. Adding the equivalent of g = py5.graphics to some preamble code worked great for run_sketch when run in static mode and for py5bot but I would need to add another hack for imported mode as well as a hack to the parsing code that is in addition to the hack I added before to make it not a reserved word. And there might be one more hack needed elsewhere. I don't like doing all of this because it will make the code brittle and I'm probably going to overlook something and regret it later.

Allowing the variable g to be reassigned while also functioning correctly as the primary graphics object across all of py5's modes causes too many problems and isn't a good solution here.

However, while investigating this, I noticed that there already is a py5.get_graphics() method. It returns the same object that py5.g accesses. Rather than add a property py5.graphics or py5.primary_graphics in addition to or replacing py5.g when the method py5.get_graphics() already exists, how about we deprecate g now and instruct people to just use get_graphics() instead? We don't need multiple ways to access the same object, especially if it will cause headaches. There's no need for g to be a reserved word and conflict with people's color-related code if people can access the same object with get_graphics().

With this plan, g will remain a reserved word during the time g is deprecated. The next release will probably be in 2-3 months after this one, when g will be removed from py5 and will be available as a regular variable name.

@villares
Copy link
Contributor

villares commented Jul 17, 2023

Behold g is dying, then g is dead, and long live g :)

Let's add to our notes we'll have to update the docs:

@hx2A
Copy link
Collaborator Author

hx2A commented Jul 18, 2023

Behold g is dying, then g is dead, and long live g :)

It had a good run!

In all seriousness, g played an important role in the evolution of py5 and imported mode. Originally, imported mode would add local variables to the namespace used to execute the user's draw() method for all the dynamic variables (mouse_x, mouse_y, etc), but g was problematic because it involved large Java object being copied again and again. I got rid of that and went with the implementation py5 has now, which ultimately was a much better design decision.

Let's add to our notes we'll have to update the docs:

I added a deprecation note to g's reference doc page. You can read it here:

http://dev.py5coding.org/reference/sketch_g.html

That is the dev website which contains everything that will be on the production site once the release goes live.

I didn't think to remove g from the summary page. Thank you for pointing this out. I will remove it right now.

Are you suggesting that we leave g in the reference docs, even after it has been removed? I hadn't thought that far ahead, and don't have an opinion about this. Do you think that is the right choice?

@hx2A hx2A mentioned this issue Jul 21, 2023
@hx2A hx2A closed this as completed in #321 Jul 21, 2023
@hx2A hx2A mentioned this issue Jul 21, 2023
3 tasks
@hx2A
Copy link
Collaborator Author

hx2A commented Jul 21, 2023

I removed this but opened #322 to track the final removal from py5.

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 a pull request may close this issue.

2 participants