Closed GeorgeEfstathiadis closed 10 months ago
Discussed this with JP, and agreed it's better this way because we have studies running their pipelines using forest. If we change this we would have to send out an alert for this to all study teams using this, which is not something we want to do right now, thus we only add the cleaner version on top of the other and just change MINUTELY to MINUTE.
The code in the develop
branch is under active development by definition and is not meant to be used in production because all the code in it is subject to change. We decided to use this particular branching model to avoid this type of situation.
How and where are these studies running? Why are they not using the stable main
branch? The teams should be able to avoid updating their version of Forest until the end of their studies so that we could continue with development?
I think it's because oak
is not part of the main release, there are a few teams that are working with accelerometer data and we have them install the develop version. Thus they are using that to run all of their data modalities.
I think for this it still makes sense to just add more options on top, because either HOURLY
or HOUR
etc. makes sense. But I agree that maybe we should discuss updating main at some point, so that develop can become again a more free change zone.
I just added pull request https://github.com/onnela-lab/forest/pull/228 and I believe the code in my branch is better suited than existing code for adding additional frequency options. The tl;dr is the 4 updated main functions for Willow, Sycamore, and Jasmine now iterate over a list of frequencies, which also stcks output into well-named folders.
The current code scrap I used is
if submits_timeframe == Frequency.HOURLY_AND_DAILY:
submits_timeframes = [Frequency.HOURLY, Frequency.DAILY]
else:
submits_timeframes = [submits_timeframe]
We can easily write a function that decomposes any of the multiple time options into their component list.
@GeorgeEfstathiadis I see. We need to be careful with what we put in production so that we can maintain development velocity and avoid unnecessary technical debt. I think JP, you, and I (and @biblicabeebli if interested) should chat about this.
For this PR lets just rename MINUTELY
to avoid confusion by adding duplicate names. We will rename the rest when it's safe to change the API.
Adding more
Frequency
names for better readability.