heart of update - will still finish tests #443

duboyal · 2024-03-18T16:37:18Z

Description

Here we have update where we use the iterator of primatry base nodes to go through each subnode of an object through the DFS method,

then our "save_node" function will take in a map of the first object , keys as uuid and values as map in the object , and compare it with the second object that we traverse, for each node in the second object that we are walking, we look up the uuid of that object, then we compare, we take the diff on the json of the subnodes , then we create appropriate patch and removes , we line up all the patch and removes and do them from last one up, based on the node logic I discussed with Ludwig .

Changes

Known Issues

Notes

Checklist

My name is on the list of contributors (CONTRIBUTORS.md) in the pull request source branch.
I have updated the documentation to reflect my changes.
My code changes have been verified by automated tests and pass all relevant test scenarios.

trunk-io · 2024-03-18T16:37:22Z

Merging to develop in this repository is managed by Trunk.

To merge this pull request, check the box to the left or comment /trunk merge below.

InnocentBug · 2024-03-27T20:11:51Z

conftest.py

@@ -50,7 +50,8 @@ def cript_api():
    """
    storage_token = os.getenv("CRIPT_STORAGE_TOKEN")

-    with cript.API(host=None, api_token=None, storage_token=storage_token, default_log_level=logging.DEBUG) as api:
+    with cript.API(host=None, api_token=None, storage_token=storage_token, default_log_level=logging.WARNING) as api:


Please leave it at debug

InnocentBug

I cannot fully follow the design idea.

I think the barebone idea is there.
This could work. I can't tell if it does or not, but it could, which is great.

For now, it would be good enough for me if this works.
Please make sure that you reenable the old tests for this.
Overwrite the old save function with your new implementation, such that the original CI/CD can pick it up.

Later down the road this will need some bigger refactoring to clean up the design.
But let's focus on making this work.

InnocentBug · 2024-03-27T20:12:03Z

conftest.py

@@ -59,6 +60,7 @@ def cript_api():
        # using the tests folder name within our cloud storage
        api._BUCKET_DIRECTORY_NAME = "tests"
        api.extra_api_log_debug_info = True
+        # api.schema.skip_validation = True


please remove

InnocentBug · 2024-03-27T20:14:27Z

src/cript/nodes/primary_nodes/material.py

-            and self.vendor is None
-        ):
-            warnings.warn(CRIPTMaterialIdentifierWarning(self))
+        #### Temporarily Disabled


Since this is a warning, not an error there is no need to disable it.

InnocentBug · 2024-03-27T20:29:24Z

tests/nodes/primary_nodes/test_material.py

+
+
+@pytest.mark.skip(reason="api")
+def test_material_property_node_add(cript_api) -> None:


This looks more like your script that you use to develop to me.
Are you sure you want this in the final test suite?

yeah so, I wanted to sort out the fixtures problem (with sending double materials ?) and use those to test ? because these are tests that I did in the interim by using API calls to create nodes instead of using the fixtures, right now I can use the fixtures but only if I don't send over repeated materials which is I'm sure also the case with API calls, but you mentioned that if I am not overriding "get_json" default args then the error should not show up, can we perhaps take a look together to determine if I am indeed overwriting these arguments? I think thats still an outstanding question we had, correct me if I'm wrong

Can you give me an expanded_json of the project that doesn't work because you get a UID, where you should have a UUID instead?
Trying to debug this.

here is the pj_node expanded json

{"node": ["Project"], "uid": "_:0d32e65d-f0af-40a3-acb7-d13dcdbe796c", "uuid": "0d32e65d-f0af-40a3-acb7-d13dcdbe796c", "updated_by": {"node": ["User"], "uid": "_:a9f97af0-8405-479e-b1f0-f99ffe51bb01", "uuid": "a9f97af0-8405-479e-b1f0-f99ffe51bb01", "created_at": "2024-03-29 11:12:43.079286", "updated_at": "2024-03-29 11:12:43.079300", "email": "test@emai.com", "model_version": "1.0.0", "orcid": "0000-0002-0000-0000", "picture": "/my/picture/path", "username": "testuser"}, "created_by": {"node": ["User"], "uid": "_:1ca76636-5cff-4ed1-9ff0-f7d8e37a0f60", "uuid": "1ca76636-5cff-4ed1-9ff0-f7d8e37a0f60", "created_at": "2024-03-29 11:12:43.079286", "updated_at": "2024-03-29 11:12:43.079300", "email": "test@emai.com", "model_version": "1.0.0", "orcid": "0000-0002-0000-0000", "picture": "/my/picture/path", "username": "testuser"}, "locked": true, "model_version": "1.0.0", "public": true, "name": "my_proj_this_time_1711725177", "notes": "my project notes", "member": [{"uid": "_:1ca76636-5cff-4ed1-9ff0-f7d8e37a0f60"}], "admin": [{"uid": "_:1ca76636-5cff-4ed1-9ff0-f7d8e37a0f60"}], "collection": [{"node": ["Collection"], "uid": "_:ddcf27eb-3d83-441b-98f8-23dc1d36bc00", "uuid": "ddcf27eb-3d83-441b-98f8-23dc1d36bc00", "name": "my complex collection name", "experiment": [{"node": ["Experiment"], "uid": "_:7cd89dea-7052-430f-8560-09b6b60060c4", "uuid": "7cd89dea-7052-430f-8560-09b6b60060c4", "name": "my experiment name"}], "doi": "10.1038/1781168a0", "citation": [{"node": ["Citation"], "uid": "_:8849331b-3aa5-4215-b422-a94b750ad365", "uuid": "8849331b-3aa5-4215-b422-a94b750ad365", "type": "reference", "reference": {"node": ["Reference"], "uid": "_:379b9383-6254-4825-9728-a1a60f8056fe", "uuid": "379b9383-6254-4825-9728-a1a60f8056fe", "type": "journal_article", "title": "Multi-architecture Monte-Carlo (MC) simulation of soft coarse-grained polymeric materials: SOft coarse grained Monte-Carlo Acceleration (SOMA)", "author": ["Ludwig Schneider", "Marcus M\u00fcller"], "journal": "Computer Physics Communications", "publisher": "Elsevier", "year": 2019, "pages": [463, 476], "doi": "10.1016/j.cpc.2018.08.011", "issn": "0010-4655", "website": "https://www.sciencedirect.com/science/article/pii/S0010465518303072"}}]}], "material": [{"node": ["Material"], "uid": "_:e8fa117c-c150-4eee-851e-f76df887bcfc", "uuid": "e8fa117c-c150-4eee-851e-f76df887bcfc", "name": "my test material 03b2e268-8d95-4617-9bf6-bf3bc0056d73", "bigsmiles": "{[][$]COC[$][]}"}]}

I am getting this error when calling save
========================================================================================================= short test summary info ========================================================================================================= FAILED nodes/primary_nodes/test_project.py::test_sending_fixtures - cript.nodes.exceptions.CRIPTNodeSchemaError: JSON database schema validation for node ['Project'] failed.Error: {'uid': '_:e8fa117c-c150-4eee-851e-f76df887bcfc'} is not valid under any of the given schemas ====================================================================================================== 1 failed, 1 warning in 36.08s ====================================================================================================== (myenv) (base) ali@Alexandras-MacBook-Air tests %

I cannot replicate this problem.
In my minimum script, I read this JSON in.
And it automatically validates it with get_json().json and that checks out fine.

So not sure, how you get this error.

import cript with cript.API(host="https://lb-stage.mycriptapp.org/") as api: project_json = '{"node": ["Project"], "uid": "_:0d32e65d-f0af-40a3-acb7-d13dcdbe796c", "uuid": "0d32e65d-f0af-40a3-acb7-d13dcdbe796c", "updated_by": {"node": ["User"], "uid": "_:a9f97af0-8405-479e-b1f0-f99ffe51bb01", "uuid": "a9f97af0-8405-479e-b1f0-f99ffe51bb01", "created_at": "2024-03-29 11:12:43.079286", "updated_at": "2024-03-29 11:12:43.079300", "email": "test@emai.com", "model_version": "1.0.0", "orcid": "0000-0002-0000-0000", "picture": "/my/picture/path", "username": "testuser"}, "created_by": {"node": ["User"], "uid": "_:1ca76636-5cff-4ed1-9ff0-f7d8e37a0f60", "uuid": "1ca76636-5cff-4ed1-9ff0-f7d8e37a0f60", "created_at": "2024-03-29 11:12:43.079286", "updated_at": "2024-03-29 11:12:43.079300", "email": "test@emai.com", "model_version": "1.0.0", "orcid": "0000-0002-0000-0000", "picture": "/my/picture/path", "username": "testuser"}, "locked": true, "model_version": "1.0.0", "public": true, "name": "my_proj_this_time_1711725177", "notes": "my project notes", "member": [{"uid": "_:1ca76636-5cff-4ed1-9ff0-f7d8e37a0f60"}], "admin": [{"uid": "_:1ca76636-5cff-4ed1-9ff0-f7d8e37a0f60"}], "collection": [{"node": ["Collection"], "uid": "_:ddcf27eb-3d83-441b-98f8-23dc1d36bc00", "uuid": "ddcf27eb-3d83-441b-98f8-23dc1d36bc00", "name": "my complex collection name", "experiment": [{"node": ["Experiment"], "uid": "_:7cd89dea-7052-430f-8560-09b6b60060c4", "uuid": "7cd89dea-7052-430f-8560-09b6b60060c4", "name": "my experiment name"}], "doi": "10.1038/1781168a0", "citation": [{"node": ["Citation"], "uid": "_:8849331b-3aa5-4215-b422-a94b750ad365", "uuid": "8849331b-3aa5-4215-b422-a94b750ad365", "type": "reference", "reference": {"node": ["Reference"], "uid": "_:379b9383-6254-4825-9728-a1a60f8056fe", "uuid": "379b9383-6254-4825-9728-a1a60f8056fe", "type": "journal_article", "title": "Multi-architecture Monte-Carlo (MC) simulation of soft coarse-grained polymeric materials: SOft coarse grained Monte-Carlo Acceleration (SOMA)", "author": ["Ludwig Schneider", "Marcus M\u00fcller"], "journal": "Computer Physics Communications", "publisher": "Elsevier", "year": 2019, "pages": [463, 476], "doi": "10.1016/j.cpc.2018.08.011", "issn": "0010-4655", "website": "https://www.sciencedirect.com/science/article/pii/S0010465518303072"}}]}], "material": [{"node": ["Material"], "uid": "_:e8fa117c-c150-4eee-851e-f76df887bcfc", "uuid": "e8fa117c-c150-4eee-851e-f76df887bcfc", "name": "my test material 03b2e268-8d95-4617-9bf6-bf3bc0056d73", "bigsmiles": "{[][$]COC[$][]}"}]}' proj = cript.load_nodes_from_json(nodes_json=project_json)

Output:

INFO: Loading node validation schema from https://lb-stage.mycriptapp.org/schema/ INFO: Loading node validation schema from https://lb-stage.mycriptapp.org/schema/ was successful. INFO: Validating Project graph 'my_proj_this_time_1711725177' ... (Can be disabled by setting `cript_api.schema.skip_validation = True`.)

InnocentBug · 2024-03-27T20:29:49Z

tests/nodes/primary_nodes/test_material.py

+
+
+@pytest.mark.skip(reason="api")
+def test_material_property_node_change(cript_api) -> None:


InnocentBug · 2024-03-27T20:31:04Z

tests/nodes/primary_nodes/test_project.py

+
+
+@pytest.mark.skip(reason="api")
+def test_update_project_change_or_reset_newly_made_materials(cript_api) -> None:


this also looks like your debug script more then a full test.

InnocentBug · 2024-03-27T20:51:32Z

src/cript/api/api.py

+                modified = json.loads(node.get_json().json)  # Assuming this is already a dictionary
+
+                # cleaned_original = self.remove_keys_from_dict(original)
+                cleaned_modified = API.remove_keys_from_dict(modified)


better names.

Something like old and new.

Cleaned and orignal don't make that much sense to me.

InnocentBug · 2024-03-27T20:52:20Z

src/cript/api/api.py

+                data = dict(diff_dict)
+
+                if data:
+                    patches = API.extract_patches(data, cleaned_modified=cleaned_modified)


of what type are these?

Would it make sense to implement them as a helper class?

InnocentBug · 2024-03-27T20:52:49Z

src/cript/api/api.py

+
+                    grouped_data = {}  # This is needed to make a group by
+
+                    for item in data:


yes, this should def. be a class from a design perspective.

InnocentBug · 2024-03-27T20:53:32Z

src/cript/api/api.py

+                        if "payload_json_removes" in item:
+                            grouped_data[key]["payload_json_removes"].update(item["payload_json_removes"])
+
+                    final_data = next(iter(grouped_data.values()))


wait what does this line do?

InnocentBug · 2024-03-27T20:55:19Z

src/cript/api/api.py

+            "parent_uuid": parent_uuid0,
+            "payload_json_patch": {},
+        }
+        pattern = re.compile(r"root(\['\w+'\]\[\d+\])+\['(\w+)'\]")


better name. pattern is not good.
And please explain in a comment every regex

heart of update - will still finish tests

fa2cdfc

duboyal added 11 commits March 18, 2024 13:09

removing unused functions

5f4e7de

cript_api.schema.skip_validation

925acb8

changes for ludwig to check

6a6e48b

rebase manually add changes

1bca45c

change to warning

40b6af2

rebase

639bb1b

commented out prints and will clean them

1c41590

cleaning comments

1802de6

trunk changes

5e4761f

skip api test for cicd

7aeee99

more trunks

9eeb2af

InnocentBug reviewed Mar 27, 2024

View reviewed changes

InnocentBug requested changes Mar 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

heart of update - will still finish tests #443

heart of update - will still finish tests #443

duboyal commented Mar 18, 2024

trunk-io bot commented Mar 18, 2024

InnocentBug Mar 27, 2024

InnocentBug left a comment

InnocentBug Mar 27, 2024

InnocentBug Mar 27, 2024

InnocentBug Mar 27, 2024

duboyal Mar 28, 2024

InnocentBug Mar 29, 2024

duboyal Mar 29, 2024

InnocentBug Mar 29, 2024

InnocentBug Mar 27, 2024

InnocentBug Mar 27, 2024

InnocentBug Mar 27, 2024

InnocentBug Mar 27, 2024

InnocentBug Mar 27, 2024

InnocentBug Mar 27, 2024

InnocentBug Mar 27, 2024



		@pytest.mark.skip(reason="api")
		def test_material_property_node_add(cript_api) -> None:



		@pytest.mark.skip(reason="api")
		def test_material_property_node_change(cript_api) -> None:



		@pytest.mark.skip(reason="api")
		def test_update_project_change_or_reset_newly_made_materials(cript_api) -> None:


		grouped_data = {} # This is needed to make a group by

		for item in data:

heart of update - will still finish tests #443

Are you sure you want to change the base?

heart of update - will still finish tests #443

Conversation

duboyal commented Mar 18, 2024

Description

Changes

Known Issues

Notes

Checklist

trunk-io bot commented Mar 18, 2024

Choose a reason for hiding this comment

InnocentBug left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment