-
Notifications
You must be signed in to change notification settings - Fork 22
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
Discussion: project stucture #7
Comments
In general looks good. Splitting metrics into modules according to the observed variable poses a danger of duplicating similar calculations twice. One of the examples - Would be nice to add a |
That's a good example indeed: In my opinion there should not be 1 method that calculates time in zone for both power and heartrate. This should be handled by separate methods (in my example e.g. heartrate.time_in_zone and power.time_in_zone). Duplicate code for these methods should live in algorithms.utils or algorithms.metrics.utils. It's probably my fault but I still do not understand your idea for the streams... All algorithms work on array-like structures right? Why is there a separate module? Can you explain this to me? |
I don't keep streams - they are a legacy of To make sure I understand your proposal correctly:
Context-specific modules e.g. |
Sounds like a plan. And I like calling it |
This might be a little late to the party but I was looking at the current projects structure yesterday and I realized that since the algorithms are the main part of this library it's strange that they're not accessible from the root of the project. How would you feel about "unnesting" the algorithms so that |
Great you brought it up - I thought about exactly the same ;) |
I guess I have to give you credit for changing my mind from wdf-centric to algorithm-centric 😛 |
(involving @sladkovm)
A lot of different algorithms, tools and models will be added so I think is good to already think about where everything is going so we don't end up with a labyrinth. I like this guide on structuring Python projects.
I think the structure below could work but maybe I'm missing things or you know a better structure.
The text was updated successfully, but these errors were encountered: