Thick Wrappers #9
Conversation
Added Poop to README for example
Static and helper folders added. Main.py added
Did pre-commit lint
Fixed Linting Problems
Created function that take binary in and spits out the ascii version
FInxed linter
Parse EXIF for original image size
Project Feature Complete
Handle RGBA images
Fix test error
There was a problem hiding this comment.
Hey there, Thick Wrappers! Thank you for participating in this year's code jam, and congratulations on making it this far! I've enjoyed reviewing your submission, and I hope that my comments help you improve as Python developers.
I also hope that you'll take my suggestions for improvement in stride. I am overall impressed with what you came up with, especially among a team of strangers in so short a time.
I think the project you came up with is a fun idea, and a great application of image processing and the theme. I also appreciate that contributes are reasonably balanced among all the team members.
We'll see you at the live stream!
|
|
||
| # Iterates through each binary output and converts each binary to ASCII, simultaneously checking for the delimiter | ||
| for binary in pixel_list: | ||
| if word_total[-5:] == ",,,..": |
There was a problem hiding this comment.
I would expect ',,,..' to be defined once as a constant, so that you can change the delimiter in one place without having to make any other changes elsewhere in the code base.
You could also have used the str.endswith method.
| delimiter = True | ||
| break | ||
| word_total += format(binary, "c") | ||
| return word_total[:-5], delimiter |
There was a problem hiding this comment.
Likewise, word_total[:-5] can be rewritten with a call to str.removesuffix, so that the code doesn't depend on a known length for the delimiter. It might look something like word_total.removesuffix(DELIMITER).
|
|
||
| T = TypeVar("T", int, np.signedinteger[Any]) | ||
|
|
||
| EXIF_MODEL_PATTERN = re.compile(r"I(?P<width>\d*)P(?P<height>\d*)P") |
There was a problem hiding this comment.
I love to see regular expressions with named groups 😄
| for y in range(height): | ||
| for x in range(width): |
There was a problem hiding this comment.
This nested loop could be flattened with itertools.product
for y, x in product(range(height), range(width)):
...| This function encrypts a string in an image. Non-ASCII characters are | ||
| stripped from the string, and a copy of the image is made. |
There was a problem hiding this comment.
I recommend this library for converting unicode-only characters to their nearest ascii equivalent: https://pypi.org/project/Unidecode/
| # GUI Contents | ||
|
|
||
| # Create Filepaths object to keep track of file paths | ||
| file_paths = Filepaths() |
There was a problem hiding this comment.
It looks like the functions above are reading this variable from the global scope. While you've written this in such a way that that won't cause any problems, it's a good practice to define global variables before any code that refers to them.
| # Add static files folder | ||
| app.add_static_files("/static", "static") | ||
| # Add dark mode config | ||
| dark_mode = ui.dark_mode() |
There was a problem hiding this comment.
Thank you for including dark mode 😄
| encrypt_text_button = ui.button( | ||
| "Encrypt", | ||
| on_click=lambda e: encrypt_event( | ||
| e, | ||
| dropdown_text_or_image.value, | ||
| enter_text_or_upload.value, | ||
| ( | ||
| text_to_encrypt | ||
| if isinstance(text_to_encrypt, str) | ||
| else text_to_encrypt.value | ||
| ), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
I appreciate that you've written tests and that you're using pytest instead of unittest.
|
|
||
| seed(a=1) | ||
| ascii = string.ascii_letters + string.digits + string.punctuation + "\n\t\r" | ||
| non_ascii = "".join(map(chr, range(128, 1000))) |
There was a problem hiding this comment.
Tasteful use of map. (I dislike seeing list(map(...)) or maps involving lambdas.)
No description provided.