Open wholmgren opened 4 years ago
ivtools
is another option. I think of iotools
as functions to handle i/o of timeseries weather data specifically. Although I suppose retrieve_sam
fetches inverter parameters as well, so ivtools
isn't a perfect fit either.
I'm in favor of moving it to iotools. I'd like to consider splitting that function into two, to read module and inverter parameters separately.
I'm not sure what the practical advantage of splitting the function by module/inverter would be. The API would remain the same in that you'd have to specify a string for the specific module/inverter database. I guess we could have different functions for each database.
@cwhanse do you have any further thoughts on how to refactor this function when moving to iotools
?
The adrinverter
is a bad fit for a function name that contains "SAM", too.
So far we've named iotools
functions for the source of the data being read; that patterns suggests read_sam
(I've never liked retrieve
). I agree if we keep sam
in the name it should only read files from sam
, so adr
would need it's own read function. We could do away with the name
argument and return all the library files, just tossing that out.
An alternative, we break the pattern and align read functions to the models, which may be more easily recognized by users, e.g., read_cec_modules
, than read_sam
.
I like read_cec_modules
better than read_sam
. I'm indifferent on retrieve
vs. read
.
This would also be a great opportunity to rename parameters to standardization with pvterms.
I like the consistency of read_
and the discoverability of e.g. _cec_modules
. How about module names? Might need a couple of them depending on the name. Ideas: cec.py
, adr.py
, sandia.py
, sam.py
, module_inverter_parameters.py
I agree that it makes sense for new functions to return new keys.
What do people think about moving
pvsystem.retrieve_sam
into theiotools
subpackage? First brought up in #436.