Significant Improvement on GASP Color System #14

RichardMartinez commented 2021年04月26日 20:22:15 +02:00

I was messing with the Color system in GASP and felt that there was room for improvement. I had used GASP's RGB colors from GASP Pygame and wanted to try integrating both RGB and HEX into the same system.

Here is the link to my improved version of GASP's color.py:
https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color_improved.py

And now to explain why it's better. To start, let's look at the current color system:

>>> from gasp import *
>>> color.BLACK
'#000000'
>>> color.black
AttributeError: module 'gasp.color' has no attribute 'black'

As shown here, the current color system is entirely case sensitive. This isn't too big of a problem if you know about it but I thought it would be more seamless if you could do this:

>>> from color_improved import *
>>> color.BLACK
'#000000'
>>> color.black
'#000000'
>>> color.bLaCk
'#000000'

This is a nice touch so I added it everywhere in my new version. Yes, every single method call in color_improved.py is NOT case-sensitive. This includes getting colors, setting the mode, and converting between systems.

In color_improved.py, to use a color object:

>>> from color_improved import *
>>> rgb_color = Color("RGB")
>>> hex_color = Color("HEX")
>>> rbg_color.black
(0, 0, 0)
>>> hex_color.black
'#000000'

To change between HEX and RGB, use set_mode():

>>> from color_improved import *
>>> color.black
'#000000'
>>> color.set_mode("RGB")
>>> color.black
(0, 0, 0)
>>> color.set_mode("HEX")
>>> color.black
'#000000'

This, of course, works with all 138 colors currently available in the color system and will still work should any new colors be added. Speaking of available, to see a full list of all available colors:

>>> from color_improved import *
>>> color.available()
[('ALICEBLUE', '#F0F8FF'), ('ANTIQUEWHITE', '#FAEBD7'), ('AQUA', '#00FFFF'), ...
>>> color.set_mode("RGB")
>>> color.available()
[('ALICEBLUE', (240, 248, 255)), ('ANTIQUEWHITE', (250, 235, 215)), ('AQUA', (0, 255, 255)), ...

Finally, there are static methods for converting RGB to HEX and vice versa. Since they are static methods, they don't require a color object to be instantiated, but will still work on an existing color object.

>>> from color_improved import *
>>> color.hex_to_rgb("#cd8234")
(205, 130, 52)
>>> color.hex_to_rgb("CD8234")
(205, 130, 52)
>>> color.rgb_to_hex((0, 0, 0))
'#000000'
>>> color.rgb_to_hex(0, 0, 0)
'#000000'

As shown, these methods were specifically designed to be error proof. In hex_to_rgb, the leading # is optional and letters are NOT case-sensitive (Nothing special about #CD8234 just an example). As for rgb_to_hex, it is designed to allow either a single passed three tuple or exactly three passed parameters. This is to avoid confusion among users who prefer one method over the other.

The last thing I was deciding on was whether to have two pre-defined color objects: "rgb_color" and "hex_color" or just the one "color". I commented out the first two but they are still there should you decide to use that idea. I ended up going with just the one "color" with it defaulting to HEX. The reason I chose this was to avoid breaking old GASP programs with a new name (hex_color) for essentially the same thing.

Overall, this version of color.py would be a significant improvement over what it currently is. Please feel free to plop it straight into the GASP codebase today. If something breaks or you have a question please let me know.

Looking forward to see how this helps!

I was messing with the Color system in GASP and felt that there was room for improvement. I had used GASP's RGB colors from GASP Pygame and wanted to try integrating both RGB and HEX into the same system. Here is the link to my improved version of GASP's color.py: https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color_improved.py And now to explain why it's better. To start, let's look at the current color system: ``` >>> from gasp import * >>> color.BLACK '#000000' >>> color.black AttributeError: module 'gasp.color' has no attribute 'black' ``` As shown here, the current color system is entirely case sensitive. This isn't too big of a problem if you know about it but I thought it would be more seamless if you could do this: ``` >>> from color_improved import * >>> color.BLACK '#000000' >>> color.black '#000000' >>> color.bLaCk '#000000' ``` This is a nice touch so I added it everywhere in my new version. Yes, every single method call in color_improved.py is NOT case-sensitive. This includes getting colors, setting the mode, and converting between systems. In color_improved.py, to use a color object: ``` >>> from color_improved import * >>> rgb_color = Color("RGB") >>> hex_color = Color("HEX") >>> rbg_color.black (0, 0, 0) >>> hex_color.black '#000000' ``` To change between HEX and RGB, use set_mode(): ``` >>> from color_improved import * >>> color.black '#000000' >>> color.set_mode("RGB") >>> color.black (0, 0, 0) >>> color.set_mode("HEX") >>> color.black '#000000' ``` This, of course, works with all 138 colors currently available in the color system and will still work should any new colors be added. Speaking of available, to see a full list of all available colors: ``` >>> from color_improved import * >>> color.available() [('ALICEBLUE', '#F0F8FF'), ('ANTIQUEWHITE', '#FAEBD7'), ('AQUA', '#00FFFF'), ... >>> color.set_mode("RGB") >>> color.available() [('ALICEBLUE', (240, 248, 255)), ('ANTIQUEWHITE', (250, 235, 215)), ('AQUA', (0, 255, 255)), ... ``` Finally, there are static methods for converting RGB to HEX and vice versa. Since they are static methods, they don't require a color object to be instantiated, but will still work on an existing color object. ``` >>> from color_improved import * >>> color.hex_to_rgb("#cd8234") (205, 130, 52) >>> color.hex_to_rgb("CD8234") (205, 130, 52) >>> color.rgb_to_hex((0, 0, 0)) '#000000' >>> color.rgb_to_hex(0, 0, 0) '#000000' ``` As shown, these methods were specifically designed to be error proof. In hex_to_rgb, the leading # is optional and letters are NOT case-sensitive (Nothing special about #CD8234 just an example). As for rgb_to_hex, it is designed to allow either a single passed three tuple or exactly three passed parameters. This is to avoid confusion among users who prefer one method over the other. The last thing I was deciding on was whether to have two pre-defined color objects: "rgb_color" and "hex_color" or just the one "color". I commented out the first two but they are still there should you decide to use that idea. I ended up going with just the one "color" with it defaulting to HEX. The reason I chose this was to avoid breaking old GASP programs with a new name (hex_color) for essentially the same thing. Overall, this version of color.py would be a significant improvement over what it currently is. Please feel free to plop it straight into the GASP codebase today. If something breaks or you have a question please let me know. Looking forward to see how this helps!

caleb33 commented 2021年04月27日 02:37:46 +02:00

Richard,

Props to you for the initiative to write all this!

Regarding your first point, I am hesitant to agree about the case insensitivity. Python is a case sensitive language (like most programming languages) and the goal of GASP is to teach Python. X and x are not the same variable. However, it is designed for beginners and we don’t wish to make the game to inaccessible. Let’s confer with Jeff as he will be the one teaching it.

I agree that having one color defaulted to hex is the better approach. The flexibility in inputs for rgb_to_hex and hex_to_rgb is well done :)

Notes/suggestions on your code:
You should not be using assert statements in code you intend for production—assert is meant for debugging while in the programming process, not for error handling. Use try–except or raise instead.
Also, readability is occasionally an issue. Your comments are helpful and descriptive and most variable names are properly chosen, but the list comprehension is better off as a traditional for-loop due to its verbosity.

I’ve made two more major changes to the code: I wrote a method called get_rgb_and_hex() that returns the rgb and hex values for an inputted color name, and I modified available() to output the list of colors and values in what I believe to be a slightly more user-friendly manner.

And this is not a major issue, but the code cannot be instantly plugged into GASP as written, as the color names are no longer constants merely within the color.py file (they are now constants of two classes: RGB and HEX). This requires writing color.HEX.WHITE. Making these changes to GASP isn’t difficult, but does require modification of the source code.

Also, I've attached my updated version of the color.py file.

Let’s talk more before committing anything,

Caleb

Richard, Props to you for the initiative to write all this! Regarding your first point, I am hesitant to agree about the case insensitivity. Python is a case sensitive language (like most programming languages) and the goal of GASP is to teach Python. `X` and `x` are not the same variable. However, it is designed for beginners and we don’t wish to make the game to inaccessible. Let’s confer with Jeff as he will be the one teaching it. I agree that having one color defaulted to hex is the better approach. The flexibility in inputs for `rgb_to_hex` and `hex_to_rgb` is well done :) Notes/suggestions on your code: You should not be using assert statements in code you intend for production—`assert` is meant for debugging while in the programming process, not for error handling. Use `try–except` or `raise` instead. Also, readability is occasionally an issue. Your comments are helpful and descriptive and most variable names are properly chosen, but the list comprehension is better off as a traditional for-loop due to its verbosity. I’ve made two more major changes to the code: I wrote a method called `get_rgb_and_hex()` that returns the rgb and hex values for an inputted color name, and I modified `available()` to output the list of colors and values in what I believe to be a slightly more user-friendly manner. And this is not a major issue, but the code cannot be instantly plugged into GASP as written, as the color names are no longer constants merely within the color.py file (they are now constants of two classes: RGB and HEX). This requires writing `color.HEX.WHITE`. Making these changes to GASP isn’t difficult, but does require modification of the source code. Also, I've attached my updated version of the color.py file. Let’s talk more before committing anything, Caleb

RichardMartinez commented 2021年04月27日 05:11:40 +02:00

Caleb,

Thank you for the feedback! I looked over your version and I agree your changes are definitely an improvement. I understand the case sensitivity could be an issue and it would be easy to remove from the attribute calls but I think it should stay for anything that is passed in as a string. Just in case someone calls color.set_mode("rgb") as is confused as to why there is an error. I also really like what you did with available(), I think the formatting is much more readable; and get_rgb_and_hex() works perfectly but could use the case sensitivity treatment since it is being passed as a string. (Again just in case some one calls color.get_rgb_and_hex("blue") and is raised an error.) As for needed to write out color.HEX.WHITE, that wouldn't be necessary as calling color.WHITE while in HEX mode returns the same thing. If we use from color import * in gasp.py, you could simply write color.WHITE when coding.

I look forward to talking more!

Richard

Caleb, Thank you for the feedback! I looked over your version and I agree your changes are definitely an improvement. I understand the case sensitivity could be an issue and it would be easy to remove from the attribute calls but I think it should stay for anything that is passed in as a string. Just in case someone calls `color.set_mode("rgb")` as is confused as to why there is an error. I also really like what you did with `available()`, I think the formatting is much more readable; and `get_rgb_and_hex()` works perfectly but could use the case sensitivity treatment since it is being passed as a string. (Again just in case some one calls `color.get_rgb_and_hex("blue")` and is raised an error.) As for needed to write out `color.HEX.WHITE`, that wouldn't be necessary as calling `color.WHITE` while in HEX mode returns the same thing. If we use `from color import *` in gasp.py, you could simply write `color.WHITE` when coding. I look forward to talking more! Richard

caleb33 commented 2021年04月28日 03:14:14 +02:00

Ah I see what was causing that. When using from . import color in gasp.py the color.HEX.WHITE was necessary but using from .color import * resolves that. I've thought about it some more and I'm down to have the string parameters be treated as case-insensitive as it should save users (future students) some hassle.
I've play-tested some of the games using this version of color and haven't had any issues, so I'm ready to commit this to the source code.
Great work Richard!

Best,
Caleb

Ah I see what was causing that. When using `from . import color` in gasp.py the `color.HEX.WHITE` was necessary but using `from .color import *` resolves that. I've thought about it some more and I'm down to have the string parameters be treated as case-insensitive as it should save users (future students) some hassle. I've play-tested some of the games using this version of color and haven't had any issues, so I'm ready to commit this to the source code. Great work Richard! Best, Caleb

RichardMartinez commented 2021年05月02日 01:08:45 +02:00

Hello again,

On Friday, I presented the current version of color to Jeff and the rest of my class. After I finished, Jeff gave me some tips to improve the code more. His main idea was to remove the need to instantiate a color object like is done on line 409. I actually completed this challenge, but I don't necessarily like how complicated color had to become to accommodate.

Here is the version designed to never be instantiated:
https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color_improved.py

As you can see, everything was moved inside the one Color class. Jeff encouraged me to follow the "DRY" rule (Don't Repeat Yourself) and only store the colors as HEX, then use hex_to_rgb() when returning colors if in RGB mode.

Everything seemed to work fine except __getattr__(). When calling Color.BLACK (Note: Jeff also asked to remove all case-insensitivity), it would always return in HEX regardless of what mode I set. After some research, I found out a lot about namespaces in Python.

I learned that if an attribute is assigned in a class namespace, the __getattr__() method is never actually called. Python always checks in the class namespace first and only ever reaches the need for __getattr__() once you reach the instance namespace. Because of this, I wasn't able to use my custom method for retrieving stored colors. If only there was a dunder method I could write that would get called before looking in the class namespace. (If there is please tell me.)

This was a big issue and was the whole reason the rest of the module had to change. My solution to this issue was to actually completely re-set all of the class attributes using a for loop in set_mode(). While this does actually work, I think it makes the whole thing significantly less readable and way too complex for what I was trying to make.

I did like some of the new added simplicity in some of the methods though, so I made another version of color that I much prefer.

Here is the version I much prefer:
https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color_preferred.py

This version is a lot closer to what color is currently. In this version, I removed case-insensitivity, simplified the available() method (while adding a new names_only option), and made get_hex_and_rgb() always return in the same order (to prevent confusion).

This version does instantiate a color object (Unlike what Jeff had asked for), but I think that it just makes everything 10x easier. Easier to read, easier to write, easier to use (Zen of Python). The object is instantiated automatically so there would be no need for the end user to do anything past importing gasp.

As for the "DRY" rule, while having colors stored as both RGB and HEX in different objects does flagrantly violate this rule, I actually kind of like it. While it is more verbose, it is a lot more beginner friendly to physically see both stored formats. Plus, if anyone ever wants to take the values stored in color.py, it would be as easy as copy-paste (I know I did this same thing when using GASP Pygame).

I want to hear what Jeff and Caleb have to say about all this so please let me know.

TL;DR I made two versions of color: one designed to never be instatiated (as per Jeff's request) and one that is instantiated that I think is much better. Please let me know what you think.

Hello again, On Friday, I presented the current version of color to Jeff and the rest of my class. After I finished, Jeff gave me some tips to improve the code more. His main idea was to remove the need to instantiate a color object like is done on [line 409](https://codeberg.org/GASP/GASP_Tkinter/src/branch/main/gasp/color.py#L409). I actually completed this challenge, but I don't necessarily like how complicated color had to become to accommodate. Here is the version designed to never be instantiated: https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color_improved.py As you can see, everything was moved inside the one Color class. Jeff encouraged me to follow the "DRY" rule (Don't Repeat Yourself) and only store the colors as HEX, then use `hex_to_rgb()` when returning colors if in RGB mode. Everything seemed to work fine except `__getattr__()`. When calling `Color.BLACK` (Note: Jeff also asked to remove all case-insensitivity), it would always return in HEX regardless of what mode I set. After some research, I found out a lot about namespaces in Python. I learned that if an attribute is assigned in a class namespace, the `__getattr__()` method is never actually called. Python always checks in the class namespace first and only ever reaches the need for `__getattr__()` once you reach the instance namespace. Because of this, I wasn't able to use my custom method for retrieving stored colors. If only there was a dunder method I could write that would get called before looking in the class namespace. (If there is please tell me.) This was a big issue and was the whole reason the rest of the module had to change. My solution to this issue was to actually completely re-set all of the class attributes using a for loop in `set_mode()`. While this does actually work, I think it makes the whole thing significantly less readable and way too complex for what I was trying to make. I did like some of the new added simplicity in some of the methods though, so I made another version of color that I much prefer. Here is the version I much prefer: https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color_preferred.py This version is a lot closer to what color is currently. In this version, I removed case-insensitivity, simplified the `available()` method (while adding a new `names_only` option), and made `get_hex_and_rgb()` always return in the same order (to prevent confusion). This version does instantiate a color object (Unlike what Jeff had asked for), but I think that it just makes everything 10x easier. Easier to read, easier to write, easier to use (Zen of Python). The object is instantiated automatically so there would be no need for the end user to do anything past importing gasp. As for the "DRY" rule, while having colors stored as both RGB and HEX in different objects does flagrantly violate this rule, I actually kind of like it. While it is more verbose, it is a lot more beginner friendly to physically see both stored formats. Plus, if anyone ever wants to take the values stored in color.py, it would be as easy as copy-paste (I know I did this same thing when using GASP Pygame). I want to hear what Jeff and Caleb have to say about all this so please let me know. TL;DR I made two versions of color: one designed to never be instatiated (as per Jeff's request) and one that is instantiated that I think is much better. Please let me know what you think.

RichardMartinez commented 2021年05月02日 02:22:50 +02:00

I also made a short demo using color_preferred. This demo shows all available colors on-screen. This could be added to the tests directory of gasp.

Here is the link:
https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color_demo.py

The import statement for color_preferred is there right now, but if color_preferred was to be placed in gasp, that line could be removed as from gasp import * would import it as well.

I also made a short demo using color_preferred. This demo shows all available colors on-screen. This could be added to the [tests](https://codeberg.org/GASP/GASP_Tkinter/src/branch/main/tests) directory of gasp. Here is the link: https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color_demo.py The import statement for color_preferred is there right now, but if color_preferred was to be placed in gasp, that line could be removed as `from gasp import *` would import it as well.

ubuntourist commented 2021年05月02日 03:09:06 +02:00

For your viewing pleasure... Maybe not applicable, but it deals with some of the DRY.

NOTE: The handler for color NAMES is not implemented because I'm lazy and I think you'll be able to inject it into the code.

Run the attached as a stand-alone, and see if it is worth your time.

NOTE: I have a "standard" template that puts in a lot of stuff you can remove. The important bit is to study the normalize() function -- and fix the color name lookup.

(The try / except blocks could also probably use some improvement, but I was more interested in getting the important bits across to you.)

The hexed and r, b, g lines show how to turn the 24-bit integer (normal) into its hex representation and back to the RGB tuple triplet.

(The source and print are only to illustrate the output and not necessary. source is needed to get the columns in the print statement to line up correctly.)

For your viewing pleasure... Maybe not applicable, but it deals with some of the DRY. **NOTE:** The handler for color **NAMES** is not implemented because I'm lazy and I think you'll be able to inject it into the code. Run the attached as a stand-alone, and see if it is worth your time. **NOTE:** I have a "standard" template that puts in a lot of stuff you can remove. The important bit is to study the `normalize()` function -- and fix the color name lookup. (The `try` / `except` blocks could also probably use some improvement, but I was more interested in getting the important bits across to you.) The `hexed` and `r, b, g` lines show how to turn the 24-bit integer (`normal`) into its hex representation and back to the RGB tuple triplet. (The `source` and `print` are only to illustrate the output and not necessary. `source` is needed to get the columns in the `print` statement to line up correctly.)

ubuntourist commented 2021年05月02日 03:39:21 +02:00

P.S. Ideally, the color names would all be mapped to the 24-bit integer that it corresponds to. (A throw-away function could be written and used once, using the original colors list from the source code file as input and producing a new source code file that uses the 24-bit integers.)

Then, all three ways of calling normalize would produce the 24-bit integer.

P.S. Ideally, the color names would all be mapped to the 24-bit integer that it corresponds to. (A throw-away function could be written and used once, using the original colors list from the source code file as input and producing a new source code file that uses the 24-bit integers.) Then, all three ways of calling `normalize` would produce the 24-bit integer.

RichardMartinez commented 2021年05月02日 21:14:39 +02:00

Thanks for this idea! After studying and messing with the normalize() function, I got a decent idea of how it worked. I used a slightly modified version of normalize() that I wrote as a throwaway to make a list of normalized 24-bit integers for all colors currently in GASP. Using those new values, I wrote a short demo that could potentially be added.

Here is the link:
https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/normalize_demo.py

Note that all the values in the existing system and normalized system are the same; meaning everything worked according to plan.

It would be relatively easy to add this system into color.py and would help with the DRY. The only thing that the version loses out on is physically seeing both stored formats. This may confuse someone who is reading the GASP source code as to where the returned values come from. Although, I guess this wouldn't be an issue as they don't need to know necessarily how it works and just need to know how to use it. Otherwise, this could definitely be something that could improve the source code.

Thanks for this idea! After studying and messing with the `normalize()` function, I got a decent idea of how it worked. I used a slightly modified version of `normalize()` that I wrote as a throwaway to make a list of normalized 24-bit integers for all colors currently in GASP. Using those new values, I wrote a short demo that could potentially be added. Here is the link: https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/normalize_demo.py Note that all the values in the existing system and normalized system are the same; meaning everything worked according to plan. It would be relatively easy to add this system into color.py and would help with the DRY. The only thing that the version loses out on is physically seeing both stored formats. This may confuse someone who is reading the GASP source code as to where the returned values come from. Although, I guess this wouldn't be an issue as they don't need to know necessarily how it works and just need to know how to use it. Otherwise, this could definitely be something that could improve the source code.

RichardMartinez commented 2021年05月02日 21:33:32 +02:00

Also, I noticed that PAPAYAWHIP and PEACHPUFF both corresponded to the same values. So I wrote a short for loop that would reveal any other duplicate colors:

from color_preferred import *
list_of_color_names = color.available(names_only=True)
for color_name in list_of_color_names:
 index = list_of_color_names.index(color_name)
 for i in range(index + 1, len(list_of_color_names)):
 new_str = list_of_color_names[i]
 if color.from_str(color_name) == color.from_str(new_str):
 print(color_name, new_str, color.from_str(color_name))

After running this, this was the output:

AQUA CYAN #00FFFF
BEIGE BISQUE #F5F5DC
FUCHSIA MAGENTA #FF00FF
PAPAYAWHIP PEACHPUFF #FFEFD5

If you check the values, it is correct that these four sets of colors are duplicates. I don't know if this was intentional or not but it seems like another case of DRY.

Also, I noticed that `PAPAYAWHIP` and `PEACHPUFF` both corresponded to the same values. So I wrote a short for loop that would reveal any other duplicate colors: ``` from color_preferred import * list_of_color_names = color.available(names_only=True) for color_name in list_of_color_names: index = list_of_color_names.index(color_name) for i in range(index + 1, len(list_of_color_names)): new_str = list_of_color_names[i] if color.from_str(color_name) == color.from_str(new_str): print(color_name, new_str, color.from_str(color_name)) ``` After running this, this was the output: ``` AQUA CYAN #00FFFF BEIGE BISQUE #F5F5DC FUCHSIA MAGENTA #FF00FF PAPAYAWHIP PEACHPUFF #FFEFD5 ``` If you check the values, it is correct that these four sets of colors are duplicates. I don't know if this was intentional or not but it seems like another case of DRY.

ubuntourist commented 2021年05月02日 22:32:26 +02:00

The only thing that the version loses out on is physically seeing both stored formats. This may confuse someone who is reading the GASP source code as to where the returned values come from. Although, I guess this wouldn't be an issue as they don't need to know necessarily how it works and just need to know how to use it.

Ah. I forgot to mention that. I took care of that in my version a long time ago: I wrote a one-off that added comments to the end of each color. (But then I got side-tracked and didn't go back to work on it.) See:

https://codeberg.org/GASP/GASP_Qt/src/branch/main/gasp/color.py

So, in yours you could have lines like:

WHITE = 16777215 # (127, 127, 127) #FFFFFF

or similar, to give other coders a clue. (And, of course, a comment or two at the top explaining that the 24-bit integer is three 8-bit values all mashed together.)

> The only thing that the version loses out on is physically seeing both stored formats. This may confuse someone who is reading the GASP source code as to where the returned values come from. Although, I guess this wouldn't be an issue as they don't need to know necessarily how it works and just need to know how to use it. Ah. I forgot to mention that. I took care of that in my version a long time ago: I wrote a one-off that added comments to the end of each color. (But then I got side-tracked and didn't go back to work on it.) See: https://codeberg.org/GASP/GASP_Qt/src/branch/main/gasp/color.py So, in yours you could have lines like: ``` WHITE = 16777215 # (127, 127, 127) #FFFFFF ``` or similar, to give other coders a clue. (And, of course, a comment or two at the top explaining that the 24-bit integer is three 8-bit values all mashed together.)

RichardMartinez commented 2021年05月03日 04:08:30 +02:00

I went ahead and made a version of color that uses normalized values.

Here is the link:
https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color_normalized.py

Note that this version and color_preferred are call compatible meaning that they both would function identically and the only difference between the two would be the "back end" so to speak. I do like that color_normalized is sleeker and methods are simpler. I took your suggestion and used a throwaway function to format comments next to the normalized values.

To verify that color_normalized and color_preferred are call compatible, I wrote a short doctest.

Here is the link:
https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/test_color.txt

By running:

$ python -m doctest -v test_color.txt

you can see that all the doctests pass with color_normalized. Swapping color_normalized to color_preferred in the opening import and running the same command shows that all the doctests pass with color_preferred as well. This doctest file could also be placed in the tests directory of GASP.

I went ahead and made a version of color that uses normalized values. Here is the link: https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color_normalized.py Note that this version and color_preferred are call compatible meaning that they both would function identically and the only difference between the two would be the "back end" so to speak. I do like that color_normalized is sleeker and methods are simpler. I took your suggestion and used a throwaway function to format comments next to the normalized values. To verify that color_normalized and color_preferred are call compatible, I wrote a short doctest. Here is the link: https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/test_color.txt By running: ``` $ python -m doctest -v test_color.txt ``` you can see that all the doctests pass with color_normalized. Swapping color_normalized to color_preferred in the opening import and running the same command shows that all the doctests pass with color_preferred as well. This doctest file could also be placed in the [tests](https://codeberg.org/GASP/GASP_Tkinter/src/branch/main/tests) directory of GASP.

ubuntourist commented 2021年05月03日 14:09:54 +02:00

Looks "luv'ly".

And I really should use doctest (and unit tests, and functional tests). And decorators. But in the words of one Jeff Elkner "I'll just get a student to do that." 😉

Looks "luv'ly". And I really should use doctest (and unit tests, and functional tests). And decorators. But in the words of one Jeff Elkner "_I'll just get a student to do that."_ :wink:

RichardMartinez commented 2021年05月03日 17:25:39 +02:00

I met with Jeff and Caleb this morning and showed off what I had made. Jeff was happy I had been this ambitious but he told me about another rule: YAGNI (You ain't gonna need it). Basically, it's better to make something really simple and add more as you need it, than to try and predict future needs of the software. I had gotten way ahead of myself because tkinter (what GASP is a wrapper for) doesn't use RGB at all and only uses HEX. My original idea was that should we ever work on GASP Pygame again, it would be really easy to move the color module there. But Jeff told me that it would make more sense to cross that bridge when (or if) we get to it.

I made a much simpler version of color that is not meant to be instantiated. This ticks all the boxes and works just fine so it would slot right into GASP just fine.

Here is the link:
https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color_simple.py

To me, although it's sad to see huge portions I wrote not be implemented, I know that I learned a lot while developing this. I agree that having a simple color system makes sense for what GASP is trying to do. I also updated both color_demo.py and test_color.txt to use color_simple.

I met with Jeff and Caleb this morning and showed off what I had made. Jeff was happy I had been this ambitious but he told me about another rule: YAGNI (You ain't gonna need it). Basically, it's better to make something really simple and add more as you need it, than to try and predict future needs of the software. I had gotten way ahead of myself because tkinter (what GASP is a wrapper for) doesn't use RGB at all and only uses HEX. My original idea was that should we ever work on GASP Pygame again, it would be really easy to move the color module there. But Jeff told me that it would make more sense to cross that bridge when (or if) we get to it. I made a much simpler version of color that is not meant to be instantiated. This ticks all the boxes and works just fine so it would slot right into GASP just fine. Here is the link: https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color_simple.py To me, although it's sad to see huge portions I wrote not be implemented, I know that I learned a lot while developing this. I agree that having a simple color system makes sense for what GASP is trying to do. I also updated both color_demo.py and test_color.txt to use color_simple.

RichardMartinez commented 2021年05月06日 05:36:57 +02:00

After speaking with Jeff again today, I made what I think to be the final version of color.py. This version is very close to what was in GASP initially. It's just a module with a bunch of predefined HEX strings. (Note: I looked into it and fixed the duplicate color values that were meant to be different.) The biggest difference between this version and what color was initially is the few functions at the bottom. These functions (from_str(), available(), and rgb_to_hex()) just add bit of ease of use on top of color.

Here is the link:
https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color.py

I also went ahead and updated both color_demo.py and test_color.txt. Both now work with this version of color.

Make sure when importing this version of color, you use import color rather than from color import *. That way, you would call color.WHITE just like previous versions.

I guess if I learned anything from working on color, it's that the best solution to a problem is usually the easiest one.

After speaking with Jeff again today, I made what I think to be the final version of color.py. This version is very close to what was in GASP initially. It's just a module with a bunch of predefined HEX strings. (Note: I looked into it and fixed the duplicate color values that were meant to be different.) The biggest difference between this version and what color was initially is the few functions at the bottom. These functions (`from_str()`, `available()`, and `rgb_to_hex()`) just add bit of ease of use on top of color. Here is the link: https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color.py I also went ahead and updated both [color_demo.py](https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/color_demo.py) and [test_color.txt](https://gitlab.com/RichardMartinez/csc200/-/blob/master/gasp/color/test_color.txt). Both now work with this version of color. Make sure when importing this version of color, you use `import color` rather than `from color import *`. That way, you would call `color.WHITE` just like previous versions. I guess if I learned anything from working on color, it's that the best solution to a problem is usually the easiest one.