Skip to content

Reconfigured AbstractApplication to inherit from abc.ABC #74

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.

Sign up for GitHub

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

Open
tobsecret wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Reconfigured AbstractApplication to inherit from abc.ABC #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
320867e
reconfigured abstractmethod usage
tobsecret Apr 24, 2025
e3d5c70
more general implementation of is_overridden
tobsecret Apr 24, 2025
9dae15d
hard code is_overridden to check AbstractApplication
tobsecret Apr 24, 2025
d29effa
replace AbstractApplication() with BaseMockApplication() so tests pass
tobsecret Apr 24, 2025
4a5a6f5
missed one is_overridden replacement
tobsecret May 6, 2025
76e8269
get_command for NonSequencingApplication
tobsecret May 6, 2025
8dc846d
fix copypasta
tobsecret May 6, 2025
388b976
rename is_overridden to is_implemented
tobsecret May 6, 2025
4d32179
is_implemented is a classmethod
tobsecret May 6, 2025
77b58bb
account for classmethods vs instance methods
tobsecret May 6, 2025
90f3453
make staticmethod
tobsecret May 6, 2025
9daba61
fixed incorrect not cls.is_implemented logic
tobsecret May 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 25 additions & 19 deletions isabl_cli/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from isabl_cli.settings import system_settings


class AbstractApplication: # pylint: disable=too-many-public-methods
class AbstractApplication(abc.ABC): # pylint: disable=too-many-public-methods

"""An Abstract Isabl application."""

Expand Down Expand Up @@ -147,7 +147,7 @@ class AbstractApplication: # pylint: disable=too-many-public-methods
# -----------------------------
# USER REQUIRED IMPLEMENTATIONS
# -----------------------------

@abc.abstractmethod
def get_command(self, analysis, inputs, settings): # pylint: disable=W9008
"""
Must return a shell command for the analysis as a string.
Expand All @@ -166,7 +166,6 @@ def get_command(self, analysis, inputs, settings): # pylint: disable=W9008
# OPTIONAL IMPLEMENTATIONS
# ------------------------

@abc.abstractmethod
def get_experiments_from_cli_options(self, **cli_options): # pylint: disable=W9008
"""
Must return list of target-reference experiment tuples given the parsed options.
Expand Down Expand Up @@ -261,7 +260,6 @@ def get_project_analysis_results(self, analysis): # pylint: disable=W9008,W0613
"""
return {} # pragma: no cover

@abc.abstractmethod # add __isabstractmethod__ property to method
def merge_project_analyses(self, analysis, analyses):
"""
Merge analyses on a project level basis.
Expand Down Expand Up @@ -298,7 +296,6 @@ def get_individual_analysis_results(self, analysis): # pylint: disable=W9008,W0
"""
return {} # pragma: no cover

@abc.abstractmethod # add __isabstractmethod__ property to method
def merge_individual_analyses(self, analysis, analyses):
"""
Merge analyses on a individual level basis.
Expand Down Expand Up @@ -594,9 +591,10 @@ def application(self):
@cached_property
def individual_level_auto_merge_application(self):
"""Get or create an individual level application database object."""
assert not hasattr(
self.merge_individual_analyses, "__isabstractmethod__"
), "No logic implemented to merge analyses for an individual..."
if not self.is_implemented(self.merge_individual_analyses):
raise NotImplementedError(
"No logic implemented to merge analyses for an individual..."
)

application = api.create_instance(
endpoint="applications",
Expand All @@ -617,10 +615,11 @@ def individual_level_auto_merge_application(self):
@cached_property
def project_level_auto_merge_application(self):
"""Get or create a project level application database object."""
assert not hasattr(
self.merge_project_analyses, "__isabstractmethod__"
), "No logic implemented to merge project analyses..."

if not self.is_implemented(self.merge_project_analyses):
raise NotImplementedError(
"No logic implemented to merge analyses for a project..."
)

application = api.create_instance(
endpoint="applications",
name=f"{self.NAME} Project Application",
Expand All @@ -645,12 +644,12 @@ def assembly(self):
@property
def has_project_auto_merge(self):
"""Return True if project level auto merge logic is defined."""
return not hasattr(self.merge_project_analyses, "__isabstractmethod__")
return self.is_implemented(self.merge_project_analyses)

@property
def has_individual_auto_merge(self):
"""Return True if individual level auto merge logic is defined."""
return not hasattr(self.merge_individual_analyses, "__isabstractmethod__")
"""Return True if individual level auto merge logic is defined."""
return self.is_implemented(self.merge_individual_analyses)

@property
def _application_results(self):
Expand Down Expand Up @@ -759,10 +758,8 @@ def command(commit, quiet, **cli_options):

if force and restart:
raise click.UsageError("cant use --force and --restart together")

if not hasattr(
cls.get_experiments_from_cli_options, "__isabstractmethod__"
):

if cls.is_implemented(cls.get_experiments_from_cli_options):
tuples = pipe.get_experiments_from_cli_options(**cli_options)
else:
tuples = cls.get_experiments_from_default_cli_options(cli_options)
Expand Down Expand Up @@ -1240,6 +1237,15 @@ def _get_analysis_results(self, analysis, created=False):
# -----------------
# APPLICATION UTILS
# -----------------

@staticmethod
def is_implemented(method):
"""Checks if a method of the base class was overridden."""
method_name = method.__name__
method_func = getattr(method, "__func__", method) # accounts for classmethods vs instance methods
base_method = getattr(AbstractApplication, method_name, None)
return method_func is not base_method


@staticmethod
def get_result(*args, **kwargs): # pragma: no cover
Expand Down
38 changes: 21 additions & 17 deletions tests/test_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ class NonSequencingApplication(AbstractApplication):
NAME = str(uuid.uuid4())
VERSION = "STILL_TESTING"

def get_command(*_): # pylint: disable=no-method-argument
return "echo instantiated without an assembly"



class ExperimentsFromDefaulCLIApplication(AbstractApplication):
NAME = str(uuid.uuid4())
Expand Down Expand Up @@ -583,7 +587,7 @@ def test_engine(tmpdir):


def test_validate_is_pair():
application = AbstractApplication()
application = BaseMockApplication()
application.validate_is_pair([{"pk": 1}], [{"pk": 2}])

with pytest.raises(AssertionError) as error:
Expand All @@ -600,7 +604,7 @@ def test_validate_is_pair():
def test_validate_reference_genome(tmpdir):
reference = tmpdir.join("reference.fasta")
required = ".fai", ".amb", ".ann", ".bwt", ".pac", ".sa"
application = AbstractApplication()
application = BaseMockApplication()

with pytest.raises(AssertionError) as error:
application.validate_reference_genome(reference.strpath)
Expand All @@ -618,7 +622,7 @@ def test_validate_reference_genome(tmpdir):


def test_validate_fastq_only():
application = AbstractApplication()
application = BaseMockApplication()
targets = [{"raw_data": [], "system_id": "FOO"}]

with pytest.raises(AssertionError) as error:
Expand All @@ -645,7 +649,7 @@ def test_validate_fastq_only():


def test_validate_methods():
application = AbstractApplication()
application = BaseMockApplication()
targets = [{"technique": {"method": "FOO"}, "system_id": "FOO BAR"}]

with pytest.raises(AssertionError) as error:
Expand All @@ -655,7 +659,7 @@ def test_validate_methods():


def test_validate_pdx_only():
application = AbstractApplication()
application = BaseMockApplication()
targets = [{"custom_fields": {"is_pdx": False}, "system_id": "FOO"}]

with pytest.raises(AssertionError) as error:
Expand All @@ -665,7 +669,7 @@ def test_validate_pdx_only():


def test_validate_are_normals():
application = AbstractApplication()
application = BaseMockApplication()
targets = [
api.isablfy(
{"sample": {"category": "TUMOR", "system_id": "FOO"}, "system_id": "FOO"}
Expand All @@ -679,7 +683,7 @@ def test_validate_are_normals():


def test_validate_dna_rna_only():
application = AbstractApplication()
application = BaseMockApplication()
targets = [{"technique": {"category": "DNA"}, "system_id": "FOO"}]

with pytest.raises(AssertionError) as error:
Expand All @@ -706,7 +710,7 @@ def test_validate_species():


def test_validate_one_target_no_references():
application = AbstractApplication()
application = BaseMockApplication()
targets = [{}]
references = []
application.validate_one_target_no_references(targets, references)
Expand All @@ -719,7 +723,7 @@ def test_validate_one_target_no_references():


def test_validate_atleast_onetarget_onereference():
application = AbstractApplication()
application = BaseMockApplication()
targets = [{}]
references = [{}]
application.validate_at_least_one_target_one_reference(targets, references)
Expand All @@ -732,7 +736,7 @@ def test_validate_atleast_onetarget_onereference():


def test_validate_targets_not_in_references():
application = AbstractApplication()
application = BaseMockApplication()
targets = [{"pk": 1, "system_id": 1}]
references = [{"pk": 2, "system_id": 2}]
application.validate_targets_not_in_references(targets, references)
Expand All @@ -745,7 +749,7 @@ def test_validate_targets_not_in_references():


def test_validate_dna_tuples():
application = AbstractApplication()
application = BaseMockApplication()
targets = [{"system_id": 1, "technique": {"category": "DNA"}}]
references = [{"system_id": 2, "technique": {"category": "DNA"}}]
application.validate_dna_only(targets + references)
Expand All @@ -758,14 +762,14 @@ def test_validate_dna_tuples():


def test_validate_dna_pairs():
application = AbstractApplication()
application = BaseMockApplication()
targets = [{"pk": 1, "technique": {"category": "DNA"}}]
references = [{"pk": 2, "technique": {"category": "DNA"}}]
application.validate_dna_pairs(targets, references)


def test_validate_same_technique():
application = AbstractApplication()
application = BaseMockApplication()
targets = [{"system_id": 1, "technique": {"slug": "1"}}]
references = [{"system_id": 2, "technique": {"slug": "1"}}]
application.validate_same_technique(targets, references)
Expand All @@ -784,7 +788,7 @@ def test_validate_same_technique():


def test_validate_same_platform():
application = AbstractApplication()
application = BaseMockApplication()
targets = [{"system_id": 1, "platform": {"slug": "1"}}]
references = [{"system_id": 2, "platform": {"slug": "1"}}]
application.validate_same_platform(targets, references)
Expand All @@ -803,7 +807,7 @@ def test_validate_same_platform():


def test_validate_source():
application = AbstractApplication()
application = BaseMockApplication()
targets = [{"sample": {"system_id": "FOO", "source": "BLOOD"}}]
application.validate_source(targets, "BLOOD")

Expand Down Expand Up @@ -877,7 +881,7 @@ def test_get_experiments_from_default_cli_options(tmpdir):

def test_validate_individuals():
# Test matched analyis
matched_application = AbstractApplication()
matched_application = BaseMockApplication()

targets = [
{"system_id": 1, "sample": {"individual": {"pk": 1, "system_id": "ind1"}}}
Expand All @@ -895,7 +899,7 @@ def test_validate_individuals():
assert "Same individual required:" in str(error.value)

# Test unmatched analysis
unmatched_application = AbstractApplication()
unmatched_application = BaseMockApplication()
unmatched_application.IS_UNMATCHED = True

targets = [
Expand Down