Skip to content

Commit

Permalink
Finished and Removed TODO (#377)
Browse files Browse the repository at this point in the history
* removed and fixed some TODO comments

* upgraded `test_complex_process_node` to use fixture, removed unused fixtures, and removed copy.deep_copy()

* removed unused import `copy`
  • Loading branch information
nh916 authored Oct 30, 2023
1 parent 7d3f82f commit 6aec6f8
Show file tree
Hide file tree
Showing 9 changed files with 6 additions and 26 deletions.
1 change: 0 additions & 1 deletion src/cript/api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,6 @@ def get_vocab_by_category(self, category: VocabCategories) -> List[dict]:
response: Dict = requests.get(url=vocabulary_category_url, timeout=_API_TIMEOUT).json()

if response["code"] != 200:
# TODO give a better CRIPT custom Exception
raise APIError(api_error=str(response), http_method="GET", api_url=vocabulary_category_url)

# add to cache
Expand Down
1 change: 0 additions & 1 deletion src/cript/api/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ def __str__(self) -> str:
return error_message


# TODO refactor
class InvalidVocabulary(CRIPTException):
"""
Raised when the CRIPT controlled vocabulary is invalid
Expand Down
3 changes: 1 addition & 2 deletions src/cript/api/paginator.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,7 @@ def current_page_number(self, new_page_number: int) -> None:
if new_page_number < 0:
error_message: str = f"Paginator current page number is invalid because it is negative: " f"{self.current_page_number} please set paginator.current_page_number " f"to a positive page number"

# TODO replace with custom error
raise Exception(error_message)
raise RuntimeError(error_message)

else:
self._current_page_number = new_page_number
Expand Down
3 changes: 0 additions & 3 deletions src/cript/nodes/primary_nodes/computation_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,6 @@ def __init__(
"""
super().__init__(name=name, notes=notes, **kwargs)

# TODO validate type from vocab

if input_data is None:
input_data = []

Expand Down Expand Up @@ -273,7 +271,6 @@ def type(self, new_type: str) -> None:
-------
None
"""
# TODO check computational_process type with CRIPT controlled vocabulary
new_attrs = replace(self._json_attrs, type=new_type)
self._update_json_attrs_if_valid(new_attrs)

Expand Down
1 change: 0 additions & 1 deletion src/cript/nodes/primary_nodes/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,6 @@ def type(self, new_data_type: str) -> None:
-------
None
"""
# TODO validate that the data type is valid from CRIPT controlled vocabulary
new_attrs = replace(self._json_attrs, type=new_data_type)
self._update_json_attrs_if_valid(new_attrs)

Expand Down
2 changes: 0 additions & 2 deletions src/cript/nodes/primary_nodes/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,6 @@ def ingredient(self, new_ingredient_list: List[Any]) -> None:
-------
None
"""
# TODO need to validate with CRIPT controlled vocabulary
# and if invalid then raise an error immediately
new_attrs = replace(self._json_attrs, ingredient=new_ingredient_list)
self._update_json_attrs_if_valid(new_attrs)

Expand Down
3 changes: 0 additions & 3 deletions src/cript/nodes/supporting_nodes/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,6 @@ def __init__(self, name: str, source: str, type: str, extension: str, data_dicti

super().__init__(name=name, notes=notes, **kwargs)

# TODO check if vocabulary is valid or not
# is_vocab_valid("file type", type)

# setting every attribute except for source, which will be handled via setter
self._json_attrs = replace(
self._json_attrs,
Expand Down
1 change: 0 additions & 1 deletion tests/fixtures/subobjects.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ def complex_parameter_dict() -> dict:
return ret_dict


# TODO this fixture should be renamed because it is simple_algorithm_subobject not complex
@pytest.fixture(scope="function")
def simple_algorithm_node() -> cript.Algorithm:
"""
Expand Down
17 changes: 5 additions & 12 deletions tests/nodes/primary_nodes/test_process.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import copy
import json
import uuid

Expand Down Expand Up @@ -30,16 +29,14 @@ def test_simple_process() -> None:
assert my_process.keyword == my_process_keywords


def test_complex_process_node(complex_ingredient_node, simple_equipment_node, complex_citation_node, simple_property_node, simple_condition_node, simple_material_node, simple_process_node, complex_equipment_node, complex_condition_node) -> None:
def test_complex_process_node(complex_ingredient_node, complex_citation_node, simple_property_node, simple_material_node, simple_process_node, complex_equipment_node, complex_condition_node) -> None:
"""
create a process node with all possible arguments
Notes
-----
* indirectly tests the vocabulary as well, as it gives it valid vocabulary
"""
# TODO clean up this test and use fixtures from conftest.py

my_process_name = "my complex process node name"
my_process_type = "affinity_pure"
my_process_description = "my simple material description"
Expand All @@ -55,10 +52,6 @@ def test_complex_process_node(complex_ingredient_node, simple_equipment_node, co

my_notes = "my complex process notes"

# create complex process
citation = copy.deepcopy(complex_citation_node)
prop = cript.Property("n_neighbor", "value", 2.0, None)

my_complex_process = cript.Process(
name=my_process_name,
type=my_process_type,
Expand All @@ -69,9 +62,9 @@ def test_complex_process_node(complex_ingredient_node, simple_equipment_node, co
waste=process_waste,
prerequisite_process=[simple_process_node],
condition=[complex_condition_node],
property=[prop],
property=[simple_property_node],
keyword=my_process_keywords,
citation=[citation],
citation=[complex_citation_node],
notes=my_notes,
)
# assertions
Expand All @@ -83,9 +76,9 @@ def test_complex_process_node(complex_ingredient_node, simple_equipment_node, co
assert my_complex_process.waste == process_waste
assert my_complex_process.prerequisite_process[-1] == simple_process_node
assert my_complex_process.condition[-1] == complex_condition_node
assert my_complex_process.property[-1] == prop
assert my_complex_process.property[-1] == simple_property_node
assert my_complex_process.keyword[-1] == my_process_keywords[-1]
assert my_complex_process.citation[-1] == citation
assert my_complex_process.citation[-1] == complex_citation_node
assert my_complex_process.notes == my_notes


Expand Down

0 comments on commit 6aec6f8

Please sign in to comment.