Open apriljunge opened 7 months ago
As for now
Settings
can be created by file and arguments in parallel. In my opinion this should be split in two functions. In the current implementation it is not very logic what happens when a file and custom arguments are specified on the same time (custom arguments are currently ignored).
I agree!
I think there are two solutions:
- Create a second overloaded
Settings()
. One for the file, One with parameters. Same likePath()
- Create a second function
Settings_from_file()
I prefer the second option, as it is more clear what it does. But then
Path
andTrain
should be adapted likewise.
I do see your point about the second option, but:
I would prefer the overloaded solution for Settings()
and Train()
for the simplicity of having only to remember one function name and let the compiler do the work. A nice solution could be a forced Keyword-Argument for the file loading function to make it clear when used, e.g. Settings(file="my_mega_setting.yaml")
.
As for now
Settings
can be created by file and arguments in parallel. In my opinion this should be split in two functions. In the current implementation it is not very logic what happens when a file and custom arguments are specified on the same time (custom arguments are currently ignored).I think there are two solutions:
1) Create a second overloaded
Settings()
. One for the file, One with parameters. Same likePath()
2) Create a second functionSettings_from_file()
I prefer the second option, as it is more clear what it does. But then
Path
andTrain
should be adapted likewise.