search

simonarvin / eyeloop

EyeLoop is a Python 3-based eye-tracker tailored specifically to dynamic, closed-loop experiments on consumer-grade hardware.
GNU General Public License v3.0
485 stars 67 forks source link

Move code under a common folders/packages folder #3

Closed kinow closed 4 years ago

kinow commented 4 years ago

Hi,

I'm trying to understand the code base, but the code appears to be spread in multiple folders/packages. eyeloop.py appears to be taking care to load files correctly, but it would probably be simpler to use the same structure most projects use. Maybe something like

That would make it easier to package it as a module too.

Thanks

simonarvin commented 4 years ago

Thank you, we will take a look at this.

cfculhane commented 4 years ago

Hi there, I'm currently working on a refactor which would address this, as well as remove the .config state that is passed around as a global variable.

I'm thinking of putting all code related modules inside of /eyeloop to fix the mentioned issue, and then try and unstick some of the classes so that they are a bit less tightly bound, which would ease dev going forward.

kinow commented 4 years ago

+1

When I cloned the project, my first step was to create a setup.py copying an existing one I use in another project. Then I was going to add numpy, python-opencv, and other dependencies as necessary. Just so I could have an editable module in PyCharm.

After the refactor you are doing, it should be super easy to finish this :tada:

Thanks!

cfculhane commented 4 years ago

Yeah, it seems like we have similar ideas, just saw the PR to fix the index issues. Given my PR will be difficult to merge with further changes, do you mind holding off for a bit until I can reorganise the structure?

kinow commented 4 years ago

Yeah, it seems like we have similar ideas, just saw the PR to fix the index issues. Given my PR will be difficult to merge with further changes, do you mind holding off for a bit until I can reorganise the structure?

No problem at all. I can either rebase/merge, or verify and close if the issue is fixed by your new code :+1:

cfculhane commented 4 years ago

Perfect, I'll try and get something basic working in the new structure that we can then build further changes off, should have it done tonight (Australian time!)

kinow commented 4 years ago

I think that was merged @cfculhane . Feel free to revert if you'd like. I understood the basic of how that works, and can find myself again after the new code is pushed (probably).

Calling a day, but will have another look this week, and very likely keep bothering you guys for a while. Very interesting project!

done tonight (Australian time!)

Almost same timezone then, NZ time here :kiwi_fruit:

cfculhane commented 4 years ago

I think the major step of moving the files around is now complete and this issue can be closed out. Thanks for your patience @kinow and thanks for being so responsive @simonarvin!

kinow commented 4 years ago

That's great @cfculhane ! Will sync the repo today and have another look at the code base. Thanks a lot!