toonarmycaptain / dionysus Goto Github PK

When prompted to select avatar, user sees dialogue box titled 'Select .png format avatar:'

• no indication of option to use default avatar by clicking cancel, or that default avatar will be used if none is provided by user.
• No feedback of "no avatar provided, default avatar will be used", or "xyz.png" selected when avatar is provided or not.

Some feedback on as student data is entered would be ideal. eg:

Enter student name, or 'end', and hit enter: 
Tom - toms_avatar.png
Dick - No avatar provided, default avatar will be used.
Harry - harry.png

# Inform of ability to change in edit menu when this feature becomes available.

Move of data_folder_check to initialise_app.py broke test in test_app_main.py because import change didn't get carried across to the test.

Test for error if user tries to move save folder while contained image is open.
In this case probably best to provide feedback "unable to change location due to open files/images, please close any open files and try again".

• Moving later (eg on file close) will be confusing for user, as location will not move immediately if they -keep working and expect to save/find images in new location.
• Moving all except open files, or copying all files and leaving all or just open files in old location will be equally confusing, and spur bug reports when user doesn't realise image is open, and doesn't see old directory disappear.

• Decouple class functions from UI elements
• Abstract out UI elements into a UI script in UI_menus
• Add tests. Tests should be far easier when decoupled.
  #68

Currently format for .cld class data file is:

f'{student_name}, {avatar_filename}\n' for every student in the class. 

Which is bad because it has to be unnecessarily processed when reloaded. JSON would seem to be a much better and less error-prone solution (open to advice).
Saving as a JSON'd dict means we can directly de-JSON the dict and use it.
Pass that dict to the score-entry logic, add the scores (or None) for each student, pass to the image generation.
Nice and simple.

Using the student name as a key, the uncleaned input/real name, and the filename of the student's avatar:

class_data = { 
    'student_name_1': ['student 1 real name', 'student1_avatar_file_name'],
    'student_name_2': ['student 2 real name', 'student2_avatar_file_name'],
    'student_name_3': ['student 3 real name', 'student3_avatar_file_name'],
    'student_name_4': ['student 4 real name', 'student3_avatar_file_name'],
    'student_name_5': ['student 5 real name', 'student3_avatar_file_name'],
    'student_name_6': ['student 6 real name', 'student3_avatar_file_name'],
    }

I think the class name should be within the file (probably the first line), a JSON'd dictionary of the class data should be the second line.

• Refactor create_classlist_data
  Function create_classlist_data in class_functions will need to be refactored, which is nice, because it's a little ugly anyway.

Factor out the taking of class data into a function.
Factor out the file write.

• Tests
  We'll need test asserting the dict JSON'd and written to disc is the same as that loaded and de-JSON'd.
  A test with dummy data asserting that the data in is the same as the data reloaded from disc would be nice too, although I've never worked out how to test a while loop well.

The bot created this issue to inform you that pyup.io has been set up on this repo.
Once you have closed it, the bot will open pull requests for updates as soon as they are available.

Primary users will be Windows users. Appveyor.yml is not set up properly yet.

*Preferably test on Python 3.7, but if not available, 3.6 should be sufficient, although plan is to package with a Python 3.7 virtualenv interpreter.

*32 and 64 bit implementations also needed.

Configure Coveralls for repository.
This is help assess progress on writing tests. #8

Currently, there is no documentation to guide local installation and usage of this project. It should be added so that people can install it with ease and other developers can test it conveniently.

@toonarmycaptain Please guide me on how to accomplish this, and add the relevant labels.

I have a draft of the score entry logic in the repo - taking scores for a class to create a graph with.
The idea would be to loop through a class list and take an integer/float entry for each, with a preset max(and min?) score.
This needs to be completed and integrated.

The plan is to store the scores and parameters for each graph in a file within class_data/graph_data, so that a data set can be edited, reused, or a new graph created.
The format in which the data is saved is out of the scope of this issue (#16), but might be considered.

• User defined chart save folder
  • subfolders automatically created for each class on class_creation
  • create/check for class_save_folder in default user save folder and create if not present
  • spawn save chart with chart_name and class_save_folder default
  • change to save loc -> save_path/ + classname/ + chartname
• Prompt to select on first run, otherwise default to folder in same folder as app folder
• Add settings option in main menu, with submenu setting to change default image save folder
• Set save folder spawns a GUI directory select
• Remove app_data/image_data folder creation and path in DataFolder and references since it will not be used.
• copy dionysus_charts folder to new location on altered setting
• add tests for this new functionality:

Might be nice for user to be able to set a default avatar for each class, rather than a 'dionysus_default' avatar.
This shouldn't be too hard to implement, have a 'classname_default_avatar.png' in the class avatars folder, and a value in the class_data_file indicating a default avatar is provided. Or simply save as __classname_default_avatar, and make sure any user student names have prefixed underscores stripped or otherwise prevent user ending up with an avatar with that reserved name, then check for that name in avatars folder when default_avatar.png is the student's avatar, or/and change each student without a provided avatar to that name when a custom default avatar is provided.

When repeatedly testing locally, running the test test_can_create_empty_class will fail after the first time, because the class 'fooclass' is left in the local class registry, and so create_classlist will return a request to enter a different classname instead of the expected result.

Let's add some other tests, eg to test exactly that 'try another name' functionality, while we're there.

Prompt to allow user to go straight to creating a new chart after creating a class.

Could be done with dialogue, and if user wants to create a chart with the class, call new_graph with optional keyword arg class_name='class_name' that would filter through to assemble_chart_data, which would have something like if not class_name: class_name = select_classlist()

Implement pathlib for filepath handling.

This is the ideal on Python 3.4+ as it is less verbose.

Might a better method for handling configuration and path data be to implement a class with @Property attributes and @x.setter functions for those that may change dynamically (eg user default save paths).

https://pabloariasal.github.io/2018/11/25/python-descriptors/
https://www.hackerearth.com/practice/notes/samarthbhargav/a-design-pattern-for-configuration-management-in-python/

Matplotlib takes a string or file object when adding image to plot, not Pathlib Path object.

"TypeError: Object does not appear to be a 8-bit string path or a Python file-like object" when matplotlib tries to load the image.

Document use of semantic versioning and tagging of releases.

The build is failing tests on Windows. Locally the project works on Windows 10 using Python 3.7, but all the Appveyor builds fail on the test:

================================== FAILURES ===================================
_______________ TestDataFolder.test_generate_data_path_defaults _______________
self = <test_suite.test_data_folder.TestDataFolder testMethod=test_generate_data_path_defaults>
    def test_generate_data_path_defaults(self):
        os.chdir(os.path.join(os.getcwd(), '..'))
        for path in self.default_paths:
            path_result = DataFolder.generate_rel_path(path).as_uri()
>           assert path in path_result and os.getcwd() in path_result
E           AssertionError: assert ('/dionysus_app/app_data' in 'file:///C:/projects/dionysus/dionysus_app/app_data' and 'C:\\' in 'file:///C:/projects/dionysus/dionysus_app/app_data')
E            +  where 'C:\\' = <built-in function getcwd>()
E            +    where <built-in function getcwd> = os.getcwd
projects\dionysus\test_suite\test_data_folder.py:19: AssertionError

The os.getcwd() returns a Windows format path, and thus the test fails. Solution is to change to pathlib for the os functions.

Test if (as suspected) entering student names such as default avatar or default_avatar and providing an avatar will cause that student to use the application default avatar rather than the supplied avatar.

Potential bug here, but a class of bugs to be aware of.

Classes will be registered in the class_registry.index file when created.
This will allow selection of existing classes from menu, test for existing classname in new class creation without repeated iteration over filesystem.

On startup:

• Check for registry and check registry for consistency with data/update if incomplete/has entries without data in app setup logic of app_main.py:

  dionysus/app_main.py

  Line 28 in 5c95693

  # TODO: if the file structure already exists, check for previously created classes

  
  pathlib.iterdir will be particularly useful for this, and much less verbose than the os alternative. #10
• Cache registry in as Python dict for ease of searching.

Modify:

• Add classlist updates this dict AND registry file on disk
• Modify classlist updates this dict AND registry file on disk

class_functions.py is getting harder to follow, particularly when recent additions from the move to JSON for class data storage #25 get merged into the master branch.

Reorganise the functions so that they're easier to find. eg function definitions found after the function they're first called from, reading from the top of the script, in the order they appear in the calling function.

Another possibility is factoring some of this script into a seperate script. eg file IO, functions that deal interface with the class registry.

Add example images to README, one with image avatars, another with at least some default avatars.

Options for title in generated image - options would be a different title from the chart name, or the class then the title/chart name etc.

Best (easy if using a GUI to have default text) would be to suggest classname - chart name or something.

Some basic UI tests exist in test_create_classlist.py - wider tests are needed to ensure functionality.

Examples would include strange or no/null input, special characters, unexpected numbers.

This may reveal new issues/bugs to fix - that's great!

Improve function and module docstrings.

Add python 3.6 testing to circleci.

Commiting #88 solution breaks tests on create_classlist because on test initial run, the image_folder_setup runs before the given dialogues.

Mock existent files in test to prevent this, or/and mock input to avoid use of pexpect and necessity to run app in full to test this code.

If implementing the latter, adding such run-in-full integration tests would be a good idea.

Images save in correct folder in app data/image_data
Stretch - user to select/create folder (eg in my documents) for charts, then have subfolders for each class and save images or copies thereof there.
Or - have user pick save as folder for each image, possibly defaulting save dialogue GUI to above selected folder.

Currently format for .cld class data file is:

f'{student_name}, {avatar_filename}\n' for every student in the class. 

I think the class name should be within the file, but am open to suggestions on format.
JSON, pickle, etc.

What needs to be in the data is each student's name, together with at least the filename of the student's avatar.
Optionally, depending on what works best with the logic for passing info to graphs, would be the 'cleaned student name' which is the file name of the student's avatar, and including the relative file path with the filename of the avatar image.

Test for student with no score passing from score entry through chart data processing.
Not a huge optimisation, but unless using absent/no score students' data to display, these items don't need to be processed beyond the score-entry stage.

If for some reason user deletes a folder, app will fail when attempting to save avatars, chart data etc.
Add code to automatically create these folders if access is attempted and the folder does not exist.

Add enter 'N' as well as None to skip a avatar entry. Also note '...skip, to use default avatar'.

Catchup on writing tests for score_entry #48.

Add prompt to press return after entering student name.
Also at top of enter scores dialogue indicate instruction to press enter after entering score.

Copy and paste of filepath is awkward. Spawn OS file open dialogue would be better.

CodeFactor found an issue: The input method in Python 2 will read from standard input, evaluate and run the resulting string as python source code. This is similar, though in many ways worse, then using eval. On Python 2, use raw_input instead, input is safe in Python 3.

It's currently on:
dionysus_app\main_menu.py:41

Taking a class list/avatars and data set with graph parameters (scale/bar widths, axis etc) and plotting the avatars.
Plan is to use matplotlib eg this and this

The idea is to create essentially a bar graph, with with similar scores are stacked on top of eachother in columns - the specifics would be an option in the graph creation logic #16.
This is the rough avatar arrangement I'm imagining*:

So create a graph, iterate over the students/avatars/scores and plot the avatars. Save the image.

*Borrowed from mathblaster.com.

Self explanatory - we don't need that top or right border.

To create a graph/chart, a classlist/set of avatars*1 and associated scores will need to be assembled with some graph parameters to pass to the (probably matplotlib) image creation code.

The idea is to create essentially a bar graph, with the avatars of students with similar scores stacked on top of eachother. Possibly in Quartiles, Quintiles, or smaller brackets eg 80-85 or 80-89 5 or 10 point brackets)*2:

So create a graph, iterate over the students/avatars/scores and plot the avatars. Save the image.

Some options with graph titling, axis etc would be nice, but getting a prototype is a priority over sophistication.
The initial version probably needs a user selected title (eg classname - title), and could use 0-100 broken down into 10 point blocks as the column size (eg 0-9, 10-19...80-89, 90-100).

Open to suggestions, but possibly a class or a named-tuple with the students/scores and graph parameters might be the way to pass the data around - one one hand to the save logic, on the other to the image generation logic in #17.

Open question whether preprocessing the data into columns eg {col1: [student_a], col2, [student_b, student_c] etc so the columns could be assembled sequentially, or straight iterating over the set and placing the avatars on the columns as one would deal cards would be the better option.

*1 Default to using student name or hash of student name (or even ROT13 cipher to keep it simple in the first version) if no avatar/image is provided for a student. This will definitely be the case until logic to take and process images is in place.

*2 Borrowed from mathblaster.com.

Make avatars copy to class avatar folder.

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. #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. #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.

In the spirit of Hacktoberfest, I'd love some help writing tests.

CodeFactor found an issue: Relative import 'dionysus_app.main_menu', should be '_master.dionysus_app.main_menu'

It's currently on:
app_main.py:8
Commit cdeb052

Currently DataFolder supplies the following data paths:

APP_DATA = './dionysus_app/app_data/'

CLASS_DATA = APP_DATA + 'class_data'
IMAGE_DATA = APP_DATA + 'image_data'

CLASS_REGISTRY = APP_DATA + 'class_registry.index'

CHART_GENERATOR = './dionysus_app/chart_generator/'
DEFAULT_AVATAR = CHART_GENERATOR + 'default_avatar.png'

However the paths for avatars and chart_data inside each class' data folder are still made by joining those strings to CLASS_DATA and the classlist_name. Surely they could be generated programmatically from DataFolder with a supplied classlist_name?
That way there's a single place to alter should a change be made to the filestructure.

This will require modifications to DataFolder and everywhere that touches the file system, class_functions, tests that reference paths etc

Logic to edit a class - add/remove students, edit existing student names, and the CLI dialogue to do this.

NB This dialogue also needs to allow adding/removing/changing avatars.

Write tests for class_registry_functions.py. Some boilerplate test functions and comments are already in test_class_registry_functions.py.

Since app currently does not copy avatar images to app_data folder, even if properly sized, app will fail on encountering use supplied avatar, as the filename will not exist where the app tried to find it.

Better would be to add a if exists clause to supply default avatar in place if it doesn't exist, or indeed on any failure when adding the avatar, provided other errors were documented/traceable.