branch removal, improves coverage [merge]

[espg]

seems like we don't need it.

in reference to #120 , we can also improve this by updating the default parameters on the make_clusters function call to match what we want.

Now that we have max_clust_size, min_clust_size and opt_clust_size parameters, they can be set meaningfully. I recommend:

max_clust_size = 25 opt_clust_size = 14 min_clust_size = 4 or 7

We can specifically tune the min_clus_size and combine it to match with a test or error conditional.

What the smallest run you want to allow before aborting or deferring the day?

[demiangomez]

With less than 2 stations it is impossible to run. So that should be the bare minimum. I see that we have a logic for detecting more than one cluster, which triggers the generation of the backbone network.

[demiangomez]

From the modifications I see in this PR, I would actually remove the else branch. We need to check if the backbone is needed or not.

[espg]

https://github.com/demiangomez/Parallel.GAMIT/blob/99a7a4dc85ce9dd34af5bbe716733ef0a00c6205/pgamit/pyNetwork.py#L178-L185

what is "DDG", and do we still need this comment?

[demiangomez]

https://github.com/demiangomez/Parallel.GAMIT/blob/99a7a4dc85ce9dd34af5bbe716733ef0a00c6205/pgamit/pyNetwork.py#L178-L185

what is "DDG", and do we still need this comment?

DDG = Demián David Gómez It's an old habit of stating who wrote the comment. You can remove that comment, not relevant anymore.

[espg]

tests pass, this can be merged.

Test coverage for network module improved to 19%

(old coverage)

pgamit/pyNetwork.py        163    133    18%   53, 58, 63, 66, 74-195, 200-252, 256-307, 313-349, 356-392, 397-418

(new coverage)

pgamit/network.py          158    128    19%   53, 58, 63, 66, 74-176, 181-231, 235-286, 292-328, 335-371, 376-397 

[demiangomez]

@espg you changed the name of pyNetwork but you didn't check the rest of the code for its use. I see at least one instance in ParallelGamit.py in the com folder. If you are renaming modules, please make sure that their calls in the rest of the project are also correct.

[espg]

@demiangomez this is fixed. There was only the one instance inside of the com folder:

> cat *.py | grep pyNetwork                                                                                                             
from pgamit.pyNetwork import Network

Our automated tests already check for and flag if module names aren't correct-- but only for code within the pgamit directory, which is why the script inside the com directory didn't trigger a failed build.

[demiangomez]

@demiangomez this is fixed. There was only the one instance inside of the com folder:

> cat *.py | grep pyNetwork                                                                                                             
from pgamit.pyNetwork import Network

Our automated tests already check for and flag if module names aren't correct-- but only for code within the pgamit directory, which is why the script inside the com directory didn't trigger a failed build.

Interesting. How can we add the com folder in the checks? Is there a way?

[espg]

@demiangomez yes, I believe we can. There's two different ways, implicit and explicit checks-- I think the implicit checks may just require us adding an __init__.py file to the coms directory with the module names and/or functions. I don't usually see __init__.py declarations outside of the library code, so I'm not sure if it will behave the same way for coms as it does for the pgamit directory and subdirectories.

The other way is explicit unit tests for those files, which I know will work. Explicit checks are how I was alerted to module name errors in this PR-- you can see where the tests failed from me renaming NetClusters to cluster, first here when I didn't update the old name in all the files, and then again here on lines 47 and 48 when I misspelled the module name (clusters instead of the correct cluster). The same sort of check also told me when I renamed parameter names to make them shorter, and forgot to change them in some files.

Doing the explicit checks would mean writing unit tests for coms, which would run from the same Parallel.pgamit.tests directory, using this syntax:

import pytest
import numpy as np   # or whatever else is needed

from ...coms import thing_to_test

... any relevant unit tests go here