Nodes of single population improved

[fdchevalier]

Nodes of single population displayed as circles instead of pies.

This is related to issue #201.

Here is the line that can be added to the NEWS file:
* nodes with single populations displayed as circles instead of pies

Here is the line that can be added to the DESCRIPTION file:
person(c("Frédéric", "D."), "Chevalier", role = "ctb", email = "fcheval@txbiomed.org", comment = c(ORCID = "0000-0003-2611-8106")))

Thank you for reviewing it.

[zkamvar]

This is a fantastic first submission! Thank you for your time and contribution 😃

Aside from the specific comments, there are a couple of larger bits that needs addressing before I can merge this (which you can blame on 2016 Zhian):

1. This whole block should probably move up to the control statement on line 144, which also means that the arguments in showplot will need to be simplified (difficulty: 4/10)
2. plot_poppr_msn() needs to be able to handle the new changes via update_poppr_graph() and update_colors() (which is why the vector of colors must be named) (difficulty: 6/10)
3. tests are now failing because of the changes and need to be updated (here and here) (difficulty: 4/10)

Zhian from 2016 added this code in here and he was not exactly in the best state of mind (ramping up to finish his Ph. D.), so the refactoring he did left a bit to be desired.

Sorry for the can of worms 🙁. I can take care of anything you don't feel comfortable updating (assuming you have checked the little box on github that allows me to make changes to your branch). Just let me know.

[fdchevalier]

Hi Zhian,

Thank you for the code improvements. I didn't know about the lengths function.

I won't blame Zhian from 2016, I am already quite happy that this package exists.

I am working on point 1 (slightly buggy for now but I should solve this soon) and 2 (if 1 goes well, I think this should be OK). For point 3, I will try but not being familiar with tests, I will let you handle this if I am going nowhere.

By the way, the little box has been checked.

Fred

[fdchevalier]

Hi Zhian,

I was able to address the 3 points 🎉! I run the test on my side and everything went well. I saw Travis CI failed but I don't think this is my fault 😛. I added myself in the DESCRIPTION file but did not touch the NEWS file (version number is still the current one).

You can edit the commits if you want.

Fred

[zkamvar]

Hi Fred,

This is fantastic! Thank you for tackling everything (especially the convoluted tests and color replacement functions).

I run the test on my side and everything went well. I saw Travis CI failed but I don't think this is my fault.

Yeah, this is a weird thing that I cannot reproduce on my machine OR appveyor, so it seems to be travis-specific.

When I get some time today, I'll check the code, add the NEWS and then merge the branch. Thank you for the contribution :)