uqrmaie1 / admixtools

https://uqrmaie1.github.io/admixtools
71 stars 14 forks source link

keyword argument typo lack of error #66

Open laufran opened 4 months ago

laufran commented 4 months ago

Hi there,

While running find_graphs, I've had some minor typos while specifying keyword arguments (e.g. giving num_admix instead of numadmix, num_start instead of numstart. The inconsistency with some arguments having underscores between words, and others lacking it doesn't make it easy). I haven't received any errors indicating that I've supplied an unrecognized keyword argument, nor do I receive some kind of log, or confirmation of what argument values I'm running under to the REPL. It's only due to my double checking that I've realized my typos. This seems like it could be a large problem, especially if there's a typo in a critical argument like numadmix, that there's no error or warning that the user is supplying an unknown keyword argument.

For reference, I'm running admixtools 2.0.0 commit 669f4ff and I'm running a command like so:

graphs = find_graphs(f2_blocks, outpop = 'chimp', stop_gen = 10000, stop_gen2 = 30, plusminus_generations = 10, num_start = 100, numadmix = 1)

in this case, where it should be numstart, not num_start.

uqrmaie1 commented 4 months ago

The inconsistency with some arguments having underscores between words, and others lacking it doesn't make it easy

I agree with that, but I think at this point it would cause more problems to change argument names than to keep them as they are

I haven't received any errors indicating that I've supplied an unrecognized keyword argument

Any R-function with the ... (ellipsis) argument has that problem. That argument is used to pass arbitrary keyword arguments on to other functions, which can be very useful, but the downside is that you won't be notified about misspelled arguments. I just saw that there is a package called ellipsis which tries to address that problem. I'll look into it, maybe I'll use it to fix the problem in the next release!

cecileane commented 4 months ago

Simply echoing the argument values actually used would be helpful. Otherwise the user has no way to double-check what values were used. Printing extra log information should not break existing code, right?

uqrmaie1 commented 4 months ago

I added checks to find_graphs() and other functions using .... They should now throw errors if unused arguments are passed to them!