Add fit_image_to_screen function in utils #150

LouisPi · 2018-12-06T07:54:05Z

DO NOT ACCEPT YET. Should finish it tonight.

Edit - resizing script should be working, see comment below.

codecov · 2018-12-06T07:56:16Z

Codecov Report

Merging #150 into devel will increase coverage by 0.72%.
The diff coverage is 92.53%.

@@            Coverage Diff             @@
##            devel     #150      +/-   ##
==========================================
+ Coverage   39.83%   40.56%   +0.72%     
==========================================
  Files         247      248       +1     
  Lines       19703    19947     +244     
==========================================
+ Hits         7849     8091     +242     
- Misses      11854    11856       +2

Impacted Files	Coverage Δ
ui/printer.py	`65.78% <100%> (+15.78%)`	⬆️
ui/utils.py	`93.8% <90.47%> (-2.03%)`	⬇️
ui/tests/test_printer.py	`75% <95.45%> (+20.45%)`	⬆️
ui/overlays.py	`97.24% <0%> (-2.76%)`	⬇️
apps/media_apps/draw/main.py	`37.43% <0%> (-1.07%)`	⬇️
apps/wifi_country/main.py	`27.02% <0%> (ø)`
ui/base_list_ui.py	`76.69% <0%> (+0.25%)`	⬆️
apps/clock/main.py	`21.31% <0%> (+0.47%)`	⬆️
ui/funcs.py	`67.79% <0%> (+1.12%)`	⬆️
ui/tests/test_listbox.py	`84.9% <0%> (+1.23%)`	⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5bf6412...82421fe. Read the comment docs.

CRImier

Mostly OK

ui/printer.py

LouisPi · 2018-12-06T16:18:58Z

Definitely not ready yet just thought I would start a pull request. Renaming variables was on my TODO as I realise the names are confusing. The typo is because I originally tried out the new Graphics Printer in an old ZPUI directory I made and had to copy everything over. Deleted the old directory now so that shouldn't happen again. Am thinking of scrapping my current technique for resizing to make an image bigger and utilising some of this (https://gist.github.com/tomvon/ae288482869b495201a0) code.

LouisPi · 2018-12-06T18:27:54Z

No tests added yet. Variable names might need clearing up to avoid confusion, comments need to be added. I have tested this as well as I can on my screen trying out multiple image sizes. Will try sort any issues listed ASAP. Help on making tests for it would be appreciated =D.

…other smaller than the correct size

…e as the screen size

LouisPi · 2018-12-09T18:00:27Z

TODO - add tests

CRImier · 2018-12-09T18:03:02Z

Ahha. There's actually a lot of code in GraphicsPrinter by now (which makes it harder to read), and I think apps/other UI elements could make use of that as well, could you maybe make it into a separate function? I.e. fit_image_to_screen(image, o).

LouisPi · 2018-12-09T18:38:12Z

Okay, I will try add that!

Where do you want it added? In printer.py still?

CRImier · 2018-12-09T18:50:18Z

That's a good question! I think it'd make sense if that code were in ui/utils.py.

LouisPi · 2018-12-09T18:52:33Z

Okay thanks.

ui/printer.py

CRImier

LGTM, but does need tests

ui/utils.py

CRImier · 2018-12-10T19:20:55Z

ui/utils.py

@@ -251,4 +254,6 @@ def fit_image_to_screen(image, o):
            top = delta // 2
            bottom = delta - top
        image = ImageOps.expand(image, border=(left, top, right, bottom), fill="black")
+        if (o.width, o.height) != image.size:
+	    logger.debug("Something strange has happened! Please contact LouisPi on the Zerophone IRC for help")


=D Better throw an exception? People won't read the logs (and with logger.debug being the lowest logging level typically not enabled by default, this will not appear in logs anyway). Also, no need to tell people to contact anyone - with each extra work they need to do, there's less and less possibility they will do it. We do have bugreporting and will be adding custom exception hooks to make reporting automatic, don't worry =) In general, very mindful of you to add this - one small thing, for runtime checks like this, it's best if you use assert, it will throw an exception by itself too!

Thanks for the feedback! Will change that tomorrow.

This reverts commit 53d2a3d.

…et image dimensions and do stuff with them

ui/tests/test_printer.py

CRImier

Mostly fine, fix the typo and I'll merge =)

ui/utils.py

…d equal dimensions compared to the image

CRImier

Looks good to me!

CRImier · 2018-12-21T20:45:38Z

Thank you!

LouisPi · 2018-12-21T20:47:28Z

No problem - you pretty much told me how to do all the tests though =D so thank you as well.

CRImier · 2018-12-21T23:48:42Z

Well, there's no tutorial for ZP devs on that yet =) Also, it's not something people do a lot, unfortunately.

first changes to GraphicsPrinter

175a3f2

CRImier approved these changes Dec 6, 2018

View reviewed changes

ui/printer.py Outdated Show resolved Hide resolved

ui/printer.py Outdated Show resolved Hide resolved

ui/printer.py Outdated Show resolved Hide resolved

ui/printer.py Outdated Show resolved Hide resolved

improved GraphicsPrinter, see comments for more info

ef6b301

LouisPi added 5 commits December 7, 2018 17:01

small efficiency changes

328291c

more testing uncovered some mistakes - see code changes

e65c757

small changes to resize method

fb47081

added an elif for screen that have one side the correct size and the …

1c84358

…other smaller than the correct size

skipped the border adding script if the image size is exactly the sam…

7ec1c2a

…e as the screen size

LouisPi added 2 commits December 9, 2018 20:22

reverted changes

31e92d9

completely untested code to add fit_image_to_screen

bd0108a

LouisPi changed the title ~~first changes to GraphicsPrinter~~ Add fit_image_to_screen function in utils Dec 9, 2018

LouisPi added 2 commits December 10, 2018 17:51

a few small changes but not working yet

09cfee8

a few small changes but not working yet

636fd50

CRImier reviewed Dec 10, 2018

View reviewed changes

ui/printer.py Outdated Show resolved Hide resolved

LouisPi added 6 commits December 10, 2018 18:53

change suggested by CRImea to make it work - thanks!

81992a1

small change to documentation

ca45b7a

small change to documentation

f927837

start of logging and typo fixed

78f9322

more logging added

188fb78

changes in logging

f03fa4c

CRImier reviewed Dec 10, 2018

View reviewed changes

changes in logging

7094d9d

LouisPi added 9 commits December 14, 2018 19:11

Merge remote-tracking branch 'upstream/devel' into devel

47ed79b

remove piportablerecorder branch I added

53d2a3d

Revert "remove piportablerecorder branch I added"

bfdb275

This reverts commit 53d2a3d.

my first tests - probably all wrong =D

eb39a6c

tried to add all scenarios for a 128x64 image but I will use PIL to g…

ad497ca

…et image dimensions and do stuff with them

got rid of unneeded logging

e7d10d7

trying to make the tests at least work - gotta sort out the mock output

1ad75a0

made the original tests work and added extra

ff5050f

for CRimea

9be5a33

CRImier requested changes Dec 21, 2018

View reviewed changes

ui/tests/test_printer.py Show resolved Hide resolved

ui/tests/test_printer.py Outdated Show resolved Hide resolved

ui/tests/test_printer.py Outdated Show resolved Hide resolved

ui/tests/test_printer.py Outdated Show resolved Hide resolved

LouisPi added 5 commits December 21, 2018 17:34

resolved issues

b024fd1

latest code - not quite ready

bfc54aa

fixed error

247ad9c

fixed error

0f184e7

dunno what the difference is but it says there is one =D

e28a26d

CRImier requested changes Dec 21, 2018

View reviewed changes

ui/utils.py Outdated Show resolved Hide resolved

LouisPi added 4 commits December 21, 2018 20:15

added more tests to cover every scenerio with less than, more than an…

b3f4e4b

…d equal dimensions compared to the image

fixed typo

28ada42

added basic logging

3e704dd

added more logging

82421fe

CRImier approved these changes Dec 21, 2018

View reviewed changes

CRImier merged commit 519897e into ZeroPhone:devel Dec 21, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fit_image_to_screen function in utils #150

Add fit_image_to_screen function in utils #150

LouisPi commented Dec 6, 2018 •

edited

codecov bot commented Dec 6, 2018 •

edited

CRImier left a comment

LouisPi commented Dec 6, 2018 •

edited

LouisPi commented Dec 6, 2018 •

edited

LouisPi commented Dec 9, 2018

CRImier commented Dec 9, 2018

LouisPi commented Dec 9, 2018 •

edited

CRImier commented Dec 9, 2018

LouisPi commented Dec 9, 2018

CRImier left a comment

CRImier Dec 10, 2018

LouisPi Dec 10, 2018

CRImier left a comment

CRImier left a comment

CRImier commented Dec 21, 2018

LouisPi commented Dec 21, 2018 •

edited

CRImier commented Dec 21, 2018

Add fit_image_to_screen function in utils #150

Add fit_image_to_screen function in utils #150

Conversation

LouisPi commented Dec 6, 2018 • edited

codecov bot commented Dec 6, 2018 • edited

Codecov Report

CRImier left a comment

Choose a reason for hiding this comment

LouisPi commented Dec 6, 2018 • edited

LouisPi commented Dec 6, 2018 • edited

LouisPi commented Dec 9, 2018

CRImier commented Dec 9, 2018

LouisPi commented Dec 9, 2018 • edited

CRImier commented Dec 9, 2018

LouisPi commented Dec 9, 2018

CRImier left a comment

Choose a reason for hiding this comment

CRImier Dec 10, 2018

Choose a reason for hiding this comment

LouisPi Dec 10, 2018

Choose a reason for hiding this comment

CRImier left a comment

Choose a reason for hiding this comment

CRImier left a comment

Choose a reason for hiding this comment

CRImier commented Dec 21, 2018

LouisPi commented Dec 21, 2018 • edited

CRImier commented Dec 21, 2018

LouisPi commented Dec 6, 2018 •

edited

codecov bot commented Dec 6, 2018 •

edited

LouisPi commented Dec 6, 2018 •

edited

LouisPi commented Dec 6, 2018 •

edited

LouisPi commented Dec 9, 2018 •

edited

LouisPi commented Dec 21, 2018 •

edited