Add pytests for polyconf

[khiron]

I decided to make up some simple unit tests for polyconf features as a starter that can be built upon.

You should be able to run just the tests polyconf alone from the repository root by typing pytest tests/polyconf or you can use the test running in vs-code.

I think every feature has a trivial test here. There is one failure that happens in the use of MDAnalysis.

The general strategy for building on this is if you are to add some functionality to say polymer.py, you add the test for the expected successful feature, then you code the feature and tweak it until the test passes.

[khiron]

@recombinatrix The request for review is just a matter of going to the files changed tab and looking at what has changed. The polyconf folder is the important stuff. The polytop directory is just me moving polytop down a level so polyconf can have it's own folder. Once you've looked at the code accept the merge pull request - at which point the changes will be merged into main branch

You can merge without looking at the code - it's just tests - but what would be the fun in that?

[khiron]

This additional check-in is to show the pattern of Test first development.

1. Observe that there are things that can be passed to Polymer initialization that will make the first statement

firstMonomer.residues.resids = 1 

into a problem

2. Write a test to model the behavior you expect your code to have

def test_polymer_initialization_arguments():
    '''
    Test that the Polymer class raises an error when initialized with a non-Monomer object
    '''
    with pytest.raises(ValueError):
        Polymer(None)
    with pytest.raises(TypeError):
        Polymer("not a Monomer")

3. run your tests and observe that that test fails with

>       firstMonomer.residues.resids = 1
E       AttributeError: 'NoneType' object has no attribute 'residues'

4. Modify your code to have the behavior you expect, which in this case is a defensive guard clause for the arguments passed to initialization

        # Guard clauses to ensure that firstMonomer.residues.resids is a sane call
        if firstMonomer is None:
            raise ValueError('firstMonomer must be a Monomer object')
        if not isinstance(firstMonomer, Monomer):
            raise TypeError('firstMonomer must be a Monomer object')
        
        firstMonomer.residues.resids = 1

[recombinatrix]

This is excellent. I’ve rewritten a big chunk of polyconf yesterday, so I’ll merge it in and start looking at the tests. Thank you.

…

Sent from my iPhone

On 30 Nov 2024, at 12:42, richard morris ***@***.***> wrote:  This additional check-in is to show the pattern of Test first development. Observe that there are things that can be passed to Polymer initialization that will make firstMonomer.residues.resids = 1 into a problem 2. Write a test to model the behavior you expect your code to have def test_polymer_initialization_arguments(): ''' Test that the Polymer class raises an error when initialized with a non-Monomer object ''' with pytest.raises(ValueError): Polymer(None) with pytest.raises(TypeError): Polymer("not a Monomer") run your tests and observe that that test fails with > firstMonomer.residues.resids = 1 E AttributeError: 'NoneType' object has no attribute 'residues' Modify your code to have the behavior you expect, which in this case is a defensive guard clause for the function # Guard clauses to ensure that firstMonomer.residues.resids is a sane call if firstMonomer is None: raise ValueError('firstMonomer must be a Monomer object') if not isinstance(firstMonomer, Monomer): raise TypeError('firstMonomer must be a Monomer object') firstMonomer.residues.resids = 1 — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

[khiron]

Oh damn I just committed some code in Polymer.init() that may block an automated merge. I'll revert the guard clause so you can automatically merge your code ... and then you can do step 4 above yourself.

The entire code is

    def __init__(self, firstMonomer: Monomer) -> None:
        """
        Initiate the polymer by building its first monomer. Takes a copy of its
        atoms and sets the residue resids to 1.

        Args:
            firstMonomer (MDAnalysis Universe): first monomer of the polymer
        """
        # Guard clauses to ensure that firstMonomer.residues.resids is a sane call
        if firstMonomer is None:
            raise ValueError('firstMonomer must be a Monomer object')
        if not isinstance(firstMonomer, Monomer):
            raise TypeError('firstMonomer must be a Monomer object')
        
        firstMonomer.residues.resids = 1
        self.first = firstMonomer
        self.polymer = self.first
        self.atoms = self.polymer.atoms

Note I added a typehint to the firstMonomer argument as that will be useful for any callers both to see at a glance what types the arguments are, but tooling can also use that to provide better code hints in modern IDEs.

[lunamorrow]

@khiron Ada is pretty full out writing documentation currently but I can give you a hand with developing tests for PolyConf once the last of Ada's changes are up (she still needs to reenable gencomp). I'm working on getting some documentation off the ground currently and then I'll pivot to tests.

[lunamorrow]

Merging this now as the tests cover most base functionality - enough for ensuring any small modifications made in future development don't break functionality.