-
Notifications
You must be signed in to change notification settings - Fork 64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bug in re-adding NAs in df2genind with unexpected result #96
Comments
I have pushed suggested edits to fix_df2genind branch. Next to what I suggest in this issue (changing line 289) I also added another check to coerce |
Thanks! I am just seeing this now. It seems to make sense to me at first glance, but as it is a core functionality we need to triple check everything. Can you add this toy example (or another) as a unit test using |
Smashing. |
Merged with master now. Thanks for the fix! =D |
When a table of alleles is constructed in
df2genind
(towards the end), NAs are reinserted. I suspect that part of evaluating a subsetted object is coercing of factors of characters with "numeric" names (e.g. "720"), NAs are not added to corresponding rows but rather to sequential number the factor level is assigned. First two examples and a small proof of concept that I hope are easy to follow:Take for example this small example from
?df2genind
.All is fine as rownames cannot be coerced to numeric.
Then take this small dataset. Bear with me.
I believe line 289 is responsible for this.
NA.ind[i]
is coerced to numeric and we all know what happens when we coerce a factor to numeric without an intermediate character step. Here's a proof of concept of what I think is going on.One solution would be to replace line 289 with
The issue can be also avoided if
is called prior to constructing
g
.What do you guys think? Can you think of any obvious edge cases where my suggestion wouldn't work?
This is probably related to my question on
hw.test
in pegas.With original function,
hw.test
returnsbut when I make the suggested edit to the
df2genind
function, it yieldsThe text was updated successfully, but these errors were encountered: