Skip to content
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

Added Unit Tests #7

Open
wants to merge 16 commits into
base: dev
Choose a base branch
from
Open

Added Unit Tests #7

wants to merge 16 commits into from

Conversation

ameknas-phac
Copy link
Collaborator

Added unit tests for utils, mcluster, and multi level clustering

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, we can hold off merging the PR till the other tests are completed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@kylacochrane
Copy link
Collaborator

kylacochrane commented Nov 22, 2024

Out of 21 tests, 18 are passing with pytest.

However, three tests are still failing, and they all reside in the test_GAS_matrix_splitter.py file. All three failures occur within the same function, prep_batch_ranges, in the matrix_splitter.py class:

def prep_batch_ranges(self):
        self.num_batches = int(self.num_lines / self.batch_size)
        rem = self.batch_size % self.num_batches
        ranges = []
        for i in range(0,self.num_batches):
            ranges.append(i*self.batch_size,i*self.batch_size+self.batch_size)
        if rem != 0:
            r = ranges[-1]
            r[1] = self.num_lines
        self.ranges = ranges

Error:

>           ranges.append(i*self.batch_size,i*self.batch_size+self.batch_size)
E           TypeError: list.append() takes exactly one argument (2 given)
genomic_address_service/classes/matrix_splitter.py:54: TypeError

Suggested Fix:
A possible resolution for this issue, could be to have the append() method receive a single argument, which in this case is a tuple containing both values. The corrected line in matrix_splitter.py could be:

ranges.append((i*self.batch_size, i*self.batch_size+self.batch_size))

@sgsutcliffe
Copy link

Out of 21 tests, 18 are passing with pytest.

However, three tests are still failing, and they all reside in the test_GAS_matrix_splitter.py file. All three failures occur within the same function, prep_batch_ranges, in the matrix_splitter.py class:

def prep_batch_ranges(self):
        self.num_batches = int(self.num_lines / self.batch_size)
        rem = self.batch_size % self.num_batches
        ranges = []
        for i in range(0,self.num_batches):
            ranges.append(i*self.batch_size,i*self.batch_size+self.batch_size)
        if rem != 0:
            r = ranges[-1]
            r[1] = self.num_lines
        self.ranges = ranges

Error:

>           ranges.append(i*self.batch_size,i*self.batch_size+self.batch_size)
E           TypeError: list.append() takes exactly one argument (2 given)
genomic_address_service/classes/matrix_splitter.py:54: TypeError

Suggested Fix: A possible resolution for this issue, could be to have the append() method receive a single argument, which in this case is a tuple containing both values. The corrected line in matrix_splitter.py could be:

ranges.append((i*self.batch_size, i*self.batch_size+self.batch_size))

That solved that issue! @kylacochrane 41bdffe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants