Open itaboada opened 2 years ago
After the changes by @ChrisCFTung , the **kwargs
received by firesong_simulation()
are directly passed to get_LuminosityFunction()
and get_evolution()
(note that in the latter I have implemented the support for parameterised evolutions, that is why now we pass optional arguments here as well). If I am not wrong we lost the configuration string parsing functionality that was previously suggested.
I agree with Chris that firesong_simulation()
should receive numerical values for the LF and evo parameters, rather than a configuration string embedding numbers. However, I think the configuration string would still be the best option for the CLI interface.
My suggestion the following:
firesong_simulation()
should take two distinct dictionaries to provide the parameters configuring LF and evolution, that are passed to get_LuminosityFunction()
and get_evolution()
respectively. I see no good reason to pass an unspecified set of keyword args to both methods and let them deal with parameters that are not meant for them;As an alternative, with argparse
it could be possible to pass down all unrecognised CLI arguments to firesong_simulation()
, leaving free hand to the user, but I think it would become increasingly complicate to input separately the configuration parameters with the command line.
Of course I am fine with any choice as long there's enough consensus :)
This needs to be made consistent with the rewriting of the Luminosity class.