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

Take avatar images and process them #33

Open
toonarmycaptain opened this issue Oct 14, 2018 · 13 comments
Open

Take avatar images and process them #33

toonarmycaptain opened this issue Oct 14, 2018 · 13 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@toonarmycaptain
Copy link
Owner

toonarmycaptain commented Oct 14, 2018

Code for accepting user images,and processing them for application usage.

  • Accept avatar image import
    - check is valid image file
    - check for correct size/proportion -> have a convention for cropping non-square images
    - convert to png
    - copy images to class_data/classname/avatars folder
  • Take all common image formats.
  • Stripping excess whitespace from scanned images.
  • Probably automatically convert to PNG by default?
  • Resizing to a specific resolution/range of resolutions (probably keep an image, in highest resolution, but possibly keep a copy sized for frequently used size - eg the 10% columns of a 0-100% scaled chart, to save constantly resizing on the fly). Consider that final charts will hopefully be 1920x1080 and projected. Logic for image generation #17
  • Might need to add whitespace/transparent space to keep aspect ratio but have all avatars sized equally for the purposes of spacing on charts. Logic for image generation #17

More of a UI thing: #15, #16

  • Would be nice to spawn a 'regular' choose file box with a file explorer, rather than pasting a filepath into the CLI.
  • If the file processing might generally hang more than a moment (the time from pressing ok to the time user expects to be able to supply another student's name, possibly spin off separate thread/s for processing, with a "please wait" message if user finishes entering data for the class before the images are processed.
@toonarmycaptain toonarmycaptain added Hacktoberfest help wanted Extra attention is needed enhancement New feature or request size/S and removed help wanted Extra attention is needed size/S labels Oct 14, 2018
@toonarmycaptain
Copy link
Owner Author

Initial processing of the avatar will need to validate as appropriate image file - attempt open in PIL, convert to png, handle if bad (eg is a text file*) then assume all avatars in app_data should be legitimate.

*although...a fun easter egg would be to convert txt file into an image if passed by mistake.

@WrightKD
Copy link

Hey , I did help with this project before , university just took all my free time. I am off now if you would want me to help you with this issue ?

@toonarmycaptain
Copy link
Owner Author

Fair enough :) Absolutely.
Truthfully I would appreciate it if you had a look at my chart/image generation code. It's kinda fragile/hacky, although I've tried to set it up to be called with a lot of custom/variable parameters to ease improvements/features.
Currently it really only works with the default parameters and if the avatar images passed in are 150px*150px. I'm pretty sure a lot of the spacing params will need to be varied according inclusion of borders, titles etc, which is why I'm setting them up as variables rather than constants.
It seems that the way I've got it set up, the sizing/spacing of the avatars on the graph is relative to the image, rather than an xy plane, so when the size of the xy plane in the image is changes (because of a title, axis labels, borders etc), the visual sizing/spacing changes too, but I couldn't deduce a simple relationship to size the avatars proportionally.

Since it's currently (fingers crossed), working well enough to be used, I've been taking time since Christmas break to learn more about testing, so I can work on improving the tests.

@WrightKD
Copy link

Hey , I just started playing around with dionysus and here's same things I learned :

Bugs

  • closing the generated chart to quickly breaks the app
  • If the user saves a generated chart , the .png is just a white image

Why is there two copies of the .png chart saved , one in dionysus_charts and another in chart_generator?

@toonarmycaptain
Copy link
Owner Author

I'll have to look at this tomorrow - I did have the chart saving correctly.

Re saving, I wanted to have a copy saved in dionysus_charts (ie wherever the user wants) to be accessed and used by the user, and another saved in the application's data (APP_DATA/CLASS_DATA/class_name/CHART_DATA) for use by the application - no functional reason yet, but possibly for preview or restoring deleted images.

app_data/
        class_data/
            class_name/  
                chart_data/  # chart data in text form and generated images 

@toonarmycaptain
Copy link
Owner Author

I'm not sure why closing the image breaks the app, but I did chase down why (I think) the plots are blank. Somewhere along the line I refactored the logic for building the chart, and added a save file dialogue, and the plt.show was moved before the save image code.

When I tried simply moving the plt.show() call to before the save image function call, something odd happened, it showed the image, but instead of having to hit the 'x' in the top corner before returning to the prompt, the save dialogue popped up too, and after hitting x and saving the image (in either order), it still hung.

I'm going to refactor it to save the chart data and image with the chart title, then display the image with an 'ok' button, then spawn the save as dialogue, and copy the image from the class_data save location to the user specified location.

@toonarmycaptain
Copy link
Owner Author

toonarmycaptain commented Jan 15, 2019

Should be fixed, possibly better than before (when it was working anyway).

Still need to add tests - if I'd had it adequately tested I'd have realised I broke it with some refactoring. Don't know how to test content of saved images though.
Pillow is now a dependency which I assume you'll use for this issue, so :)

I've created a separate avatar_processing branch for you to commit these changes to. That way any changes I'm making in development (eg when I add tests and fix the bugs they find...) don't affect what you're doing too much. I can always rebase it on development if needs be. :)

@WrightKD
Copy link

Ok cool. Are you open to the idea to maybe create another version of dionysus which is a web application?There's so much that we can do to improve the overall user ability of the application. Users will not need to install anything and run it straight from their browser. We could use something like Flask.

Another thing, we could maybe connect dionysus to a database to store user info and login from the terminal. I prefer MongoDB , because they provide a free cloud cluster with 512 MB that we could us for testing. MongoDB is very easy to work and pymongo is very simple syntax.

@toonarmycaptain
Copy link
Owner Author

I was already semi-planning on it, at least running it as a webapp running off the desktop, which I know flask can do without network access. :) #122
Not having to install python on a school machine is definitely a plus for many potential users.

My query with having something hosted and served is really about security and privacy. The original inspiration for this was for my wife (who's a teacher), but handling of student data is a serious issue for many people, so that has to be done right, particularly when people will upload photos and names together.

As for Mongo - I've heard of it, but never used it - I was planning on learning PostgreSQL and Flask as my next step. Can MongoDB store images well? For large images I know the filesystem is best, but I've read that at least for smaller images in Postgres, it's faster to save small images in the DB than retrieve from filesystem.

@WrightKD
Copy link

WrightKD commented Jan 18, 2019 via email

@WrightKD
Copy link

WrightKD commented Jan 18, 2019

Oh yes, Can we maybe change the shape of the students avatar to a circle ? I think it will look better

@toonarmycaptain
Copy link
Owner Author

toonarmycaptain commented Jan 18, 2019

That's not a bad idea. Go for it. Ultimately I'd love to have things like that customisable through a GUI. But my initial goal was having something functional for my wife by Christmas.

@toonarmycaptain
Copy link
Owner Author

I've been working on factoring out the UI stuff from the backend logic. I'm relatively new to webapps, but I'm wondering if we can have the app run with arguments such that it will run as a CLI app using the backend logic with CLI frontend, and as a server using the same backend logic when run as a service. Or do you think we just need to start a new repo?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants