Code refactoring - Githubissues

tyiannak / multimodal_movie_analysis

A Python Library for Multimodal Analysis of Movies and Content-based Movie Recommendation

25 stars 8 forks source link

Code refactoring #5

Closed tyiannak closed 3 years ago

tyiannak commented 3 years ago

This issue is linked to https://github.com/tyiannak/multimodal_movie_analysis/tree/Code-refactoring-%235 branch @lobracost

tyiannak commented 3 years ago

Description:

general cleanup (remove ; spaces, long lines)
more functions and smaller processMovie()
rename using python name styles
documentation / commenting

pakoromilas commented 3 years ago

@tyiannak anglesCluster function is not used across the code and doesn't work properly. Specifically scipy.spatial.distance.pdist function returns pairwise distances between vectors, but in our code it takes angles (not vectors) as arguments. Should I remove it since it's not used, or fix it, for future use, by making a different distance matrix?

tyiannak commented 3 years ago

@tyiannak anglesCluster function is not used across the code and doesn't work properly. Specifically scipy.spatial.distance.pdist function returns pairwise distances between vectors, but in our code it takes angles (not vectors) as arguments. Should I remove it since it's not used, or fix it, for future use, by making a different distance matrix?

no let's remove it if it is isnt used

pakoromilas commented 3 years ago

Done today:

Refactored the first 320 lines of code. Created some new functions and modified the existing. Wrote _doc_strings for every function and made the code more "pythonic".

tyiannak commented 3 years ago

Done today:

Refactored the first 320 lines of code. Created some new functions and modified the existing. Wrote _doc_strings for every function and made the code more "pythonic".

Thanx for the update @lobracost . seems ok until now. I would prefer using the pycharm pep8 and python formatting recommendations

pakoromilas commented 3 years ago

Done today:

Made whole code PEP8 compliant.
Began modifying process_video (aka processMovie) function. Wrote 3 new functions for 3 sub-tasks of process_video. This modification is not ready yet.

pakoromilas commented 3 years ago

Done today:

Whole code refactored and remodified. Docstrings for the functions and for the whole module where written.
process_video function remodified. 8 new functions created to run 8 subtasks

I could not run and check the analyze and script_debug functions since they use non existing files (features_all.npy, names_all.npy). Should I fix it by creating these files?

@tyiannak I think I finished the code refactoring tasks. Please take a look and tell me if everything's good and if you want something more.

tyiannak commented 3 years ago

Done today:

Whole code refactored and remodified. Docstrings for the functions and for the whole module where written.

process_video function remodified. 8 new functions created to run 8 subtasks

I could not run and check the analyze and script_debug functions since they use non existing files (features_all.npy, names_all.npy). Should I fix it by creating these files?

@tyiannak I think I finished the code refactoring tasks. Please take a look and tell me if everything's good and if you want something more.

Thank you @lobracost . It took me a while to try to remember what analyze / debug scripts are but they look silly anyway (had to do with some very quick experiment i had probably tried to do but in any case it is not sth we should keep).

So i guess you could do a pull request and then me, @apoman38 and @theopsall could review.

theopsall commented 3 years ago

You have greatly enriched the code with the docstrings. Nice work @lobracost !

tyiannak commented 3 years ago

great job merged and closing