search

skuschel / generatorpipeline

Parallelize your data-processing pipelines with just a decorator.
GNU General Public License v3.0
2 stars 3 forks source link

requirements of the `__init__` arguments of `BinSorter` and `DynamicBinSorter` are unintuitive #45

Open r-radloff opened 10 months ago

r-radloff commented 10 months ago

The accumulator classes BinSorter and DynamicBinSorter require binaccumulatorcls to be positional. The behavior should be changed to allow binaccumulatorcls as a keyword argument.

skuschel commented 7 months ago

Thanks for opening the issue! I made it positional for two reasons: 1) I did not (and still dont) like the name binaccumulatorcls. It says what it is but its still cumbersome to write and remember. Therefore I thought if its positional, we can still change the name later. 2) kwargs are most easy to understand if their default value is None. This is not the case here and I also didnt like that the user would be required to set it to some complicated class in order to reproduce the default value. Sure we can add a line, which changes any None to the Counter-class, but I feel that would make it even less intuitive.

So probably more a matter of taste. Why do you think positional is less intuitive than keyword?