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

Feature/sckan 226 #202

Merged
merged 44 commits into from
Feb 7, 2024
Merged

Feature/sckan 226 #202

merged 44 commits into from
Feb 7, 2024

Conversation

afonsobspinto
Copy link
Member

@afonsobspinto afonsobspinto commented Jan 4, 2024

Closes https://metacell.atlassian.net/browse/SCKAN-226

The following algorithm used was not tested and most likely will behave incorrectly for cases:

  • where an anatomical entity has more than one 'role' (Origin, Via, Destinaion)
  • where we have connections that jump layers

We need to see examples of how neurondm models those.

There are also cases where I can't get from the axioms the type of entity we are traversing in the partial order. For those cases I'm skipping the population and logging an error message. Again here I would need clarification from the neurondm team.

I did some local testing with mmset examples like:
image

  • Confirm that new statements are ingested ✅
    image

  • Confirm that existent statements are updated when there are changes, a note is created and the previous notes are kept ✅
    image

  • Confirm that existent statements are not updated when there are no changes ✅

  • Confirm that the ingestion is rolled back if any error occurs ✅

  • Confirm that erro_log.csv and success_log.csv are created ✅

  • Confirm upstream invalidation ✅ :

  • (invalid per se)
    image
    (invalid because of forward connection - multiple cases)
    image
    (invalid per se and also because of non-direct dependencies )
    image

@afonsobspinto afonsobspinto marked this pull request as ready for review January 9, 2024 23:58
@afonsobspinto afonsobspinto requested review from zsinnema and removed request for zsinnema and ddelpiano January 12, 2024 22:39
@afonsobspinto
Copy link
Member Author

While testing this I noticed that Notes are created in CET timezone but the frontend is displaying it in my system timezone:
image
cc @ddelpiano

@afonsobspinto
Copy link
Member Author

@zsinnema can you re-review this without considering what we are still discussing in https://metacell.atlassian.net/browse/SCKAN-248?
@ddelpiano asked me to have https://metacell.atlassian.net/browse/SCKAN-248 done in 2 PR's so that we can unblock @D-GopalKrishna and @Salam-Dalloul sooner rather than later

@afonsobspinto afonsobspinto removed the request for review from zsinnema January 30, 2024 14:09
@afonsobspinto afonsobspinto marked this pull request as draft January 30, 2024 14:09
@afonsobspinto afonsobspinto marked this pull request as ready for review February 1, 2024 19:25
@zsinnema zsinnema merged commit d84fa3a into develop Feb 7, 2024
1 check passed
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.

2 participants