After looking at possible use cases, wouldn't it make sense to allow the caller to specify a file to be written to?
Make sense, though via a convenience method. Libraries should provide basic building blocks (such as 'give me the csv string for these descriptors') in addition to less flexible but user friendly functions ('write the csv to path X or file Y').
Regardless, we were thinking of creating two methods, one that takes a list of descriptors, and one that takes a single descriptor.
The problem was using a *descriptors argument, not accepting a list verses a single descriptor. Accepting either a single value or a list can make things quite a bit nicer, for instance the 'target' argument of the DescriptorReader... https://gitweb.torproject.org/stem.git/blob/HEAD:/stem/descriptor/reader.py#...
That said, I haven't seen the code yet so do what you think is best.
Just to clarify, the include_fields and exclude_fields parameters would have default values of none
I was just using that as an example since I didn't know what you were defaulting it to. My assumption was that they'd both default to None to indicate "user didn't provide anything" and the behavior was... include_fields - default is to include everything (ie, all the fields that a descriptor has) exclude_fields - default is to exclude nothing
... and since we are taking in descriptors are a list rather than a *arg, we don't need to worry about specifying the keyword parameters.
Sorry, I'm coming up with two interpretations of the sentence "we don't need to worry about specifying the keyword parameters". If you mean...
... "we don't need those parameters to have a keyword" then no, we definitely want them to have keywords so users can pick and choose what they want to set.
... "users don't need to supply those parameters" then yup, without a *descriptors argument they'll be completely optional.
That said, if a caller doesn't specify either, all parameters would be returned.
Yup, sounds good. Defaulting to "give me a csv with all of the descriptor's attributes" makes sense.
Otherwise, it is expected that only one of these parameters would be specified by the caller.
It would be weird if the user set both, but we can easily handle it. Just remove anything in the exclude_fields from the include_fields.
Also, going back to features expected by the community, would users want a csv header to be written? Or simply a csv file?
Yup. Users will need a header so they can figure out what the fields are (otherwise adding new descriptor fields will break all of the old csvs that were only based on position). However, we might as well accept a 'header' boolean argument to let them turn it off if they want.
Cheers! -Damian