Closed misi9170 closed 6 months ago
@paulf81 @Bartdoekemeijer @ejsimley It would be great to get your input on the updated structure; happy to iterate on this a bit before merging.
I like the new folder structure! Makes a lot of sense to me.
hi @misi9170 this is looking really good! I have a few suggestions, either for this pull request, or to be flagged for the next step. But I should say I would say go ahead and make this the base v2 branch whenever you like. Here are my thoughts:
@Bartdoekemeijer ok if we delete the file df_reader_writer.py? We don't see any references in the code to these functions
Yes good to delete. This way mainly there to accommodate very large datasets, e.g., 1-second data of a couple months, that wouldn't fit in a single data file. I haven't used it and probably would require some rework anyway. Good to delete -- can always reintroduce if necessary.
I think the new structure looks good overall. I just had a few thoughts on the organization:
@ejsimley Thanks for the suggestions! I went ahead and moved tuner_utils.py, energy_ratio_utilities.py, and optimization.py to utilities/ (and renamed "tuner_utils" to "tuner_utilities" for consistency).
I decided to leave the data_processing module alone for now. I feel that the intention of this code is still distinct from utilities, which I see as a set of helper functions. However, there may be pieces of data_processing that should be moved to utilities; I'm imagining that there will be a second round of consolidation/reorganizing before v2 is released, and I'll keep that in mind.
@paulf81 I've also taken your suggestions into account. I'm going to add the sqlalchemy point to issue #168
One of the main changes we'd like to implement for FLASC v2 is a reorganization and consolidation of the repository. This includes:
This PR begins to address 1. and makes a few steps towards 3. In particular:
Some questions remain:
Notes
Tests currently fail because the CI workflow is pointing at the to-be-released Floris v4. This is intentional, and various pieces of FLASC will need to be updated to get these tests passing prior to the release of FLASC v2. However, all tests pass when Floris v3 is used instead.
Partly addresses #144 but does not close it.