Remove the superfluous parameter in extract_GP (and some other functions)

[Catarina-Alves]

Issue #104 shows what can happen when we have superfluous parameters in a function. In that case, was output_root and save_output. They do seem different but their information could be condensed.

This is especially relevant for the functions extract_GP and extract_features. As both output_root and save_output are parameters, one could be trying to save some GPs or features by setting the path they want but it is not saved as the save_output flag is False.
Sure, this is a bad/sloppy use of the function but they should be resistant to these type of errors.

I propose removing save_output and work with output_root in the following way:

• If it is None, it will not save anything
• If it is str, it is the path for the folder we want to save the GPs or the wavelets on

I don't see any disadvantage with what I am proposing besides a hard time checking were are all the functions that need this change.

[rlschuhmann]

As I mentioned in another issue, I generally approve of this. Are you planning to make this change in all feature sets or only for snfeatures.WaveletFeatures?

[Catarina-Alves]

As I mentioned in another issue, I generally approve of this. Are you planning to make this change in all feature sets or only for snfeatures.WaveletFeatures?

Yeah, I know we already talked about this. I guess the ideal would be to change all of them so there are no problems, right?

[rlschuhmann]

Yes - in my opinion if we want to do it we need to do it to all extraction methods. I believe it should be sufficient to go Ctrl+F save_output on snfeatures.py and gps.py to find all instances. I understand that there are two options for the high-level routine snfeatures.WaveletFeatures.extract_features(...) - either save nothing or all intermediate data (i.e. GPs, wavelets, PCA frame)?

[Catarina-Alves]

I have been thinking about that since we last spoke about this issue. What if for the high-level routine we input a dictionary/ array that contains exactly the information we want to save?

Eg: {output_root_gps: None, output_root_wavelets: "some/place", ...}

[rlschuhmann]

We could even offer multiple options: either a string (which signals saving all in the directory specified by it), or None (which signals nothing is saved) or the dict in which you can make everything as fine-grained as you like. This would be maximal freedom (now a user cannot save just the wavelets but not the GPs), and maximal convenience, at the expense of being a bit clunky to write.

[MichelleLochner]

Hi @Catarina-Alves I'm very supportive of making the change of condensing save_output and output_root as you originally proposed. It's one of those features that turned out not to really be useful. However, I would not encourage inputting parameters in a dictionary. My experience with this is that the possible keys in the dictionary fail to be properly documented. Having kwargs ensures that every option is always explicit. I think your original proposal is the best.

[Catarina-Alves]

Hi @Catarina-Alves I'm very supportive of making the change of condensing save_output and output_root as you originally proposed. It's one of those features that turned out not to really be useful. However, I would not encourage inputting parameters in a dictionary. My experience with this is that the possible keys in the dictionary fail to be properly documented. Having kwargs ensures that every option is always explicit. I think your original proposal is the best.

@MichelleLochner, just to make sure I understood:

• I can use the condensed version
• in the high-level routine I can have kwargs where if the user puts output_root_gp='some/path' and output_root_wavelet='other/path' and if they don't put anything it is assumed to not save anything/ save everything in a specific folder (I like the idea of saving by default)