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.

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

Jump to bottom

feat: add optional integration of GestaltMatcher/PEDIA (#399, #1125) #1249

Merged
merged 78 commits into from
Sep 27, 2024
Merged

feat: add optional integration of GestaltMatcher/PEDIA (#399, #1125) #1249

merged 78 commits into from
Sep 27, 2024

Conversation

holtgrewe
Copy link
Collaborator

No description provided.

@holtgrewe
Copy link
Collaborator Author

I just discussed this with @stolpeo and we came up with some high-level points:

  1. The model should not go into variants but rather into a new ext_gestaltmatcher module/Django app.
    Moving the model is sufficient.
  2. We don't understand whether the pathogenicity based prioritization has now been removed from the UI.
  3. Overall, we should harmonize the new names to use GestaltMatcher rather than Face in class names and gm_ rather than face_ in argument names.
  4. We don't understand the separation between GestaltMatcher and PEDIA completely yet. Please help us out here.
  5. Oliver will add some comments on the PR itself.

stolpeo
stolpeo requested changes Dec 19, 2023
View reviewed changes
Copy link
Contributor

@stolpeo stolpeo left a comment

Choose a reason for hiding this comment

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

Hi @ahujameg, looking good so far, as @holtgrewe already mentioned, create a new app and move the database code to there. Also, rename everything face to gm or gestaltmatcher (for class names). Please clarify what the value prioFace is meant for (sorry, didn't go through the logic) as it is not immediately clear to me from the naming. Also, please make the tests work :-)

config/settings/base.py Outdated Show resolved Hide resolved
variants/file_export.py Outdated Show resolved Hide resolved
variants/migrations/0096_smallvariantqueryfacescores.py Outdated Show resolved Hide resolved
variants/models/jobs.py Outdated Show resolved Hide resolved
variants/models/jobs.py Outdated Show resolved Hide resolved
variants/query_schemas.py Outdated Show resolved Hide resolved
variants/vueapp/src/components/FilterApp.vue Outdated Show resolved Hide resolved
variants/vueapp/src/components/FilterForm/PrioritizationPane.vue Outdated Show resolved Hide resolved
variants/vueapp/src/components/FilterForm/PrioritizationPane.vue Outdated Show resolved Hide resolved
variants/vueapp/src/components/FilterForm/PrioritizationPane.vue Outdated Show resolved Hide resolved
ahujameg and others added 20 commits February 7, 2024 14:57
@ahujameg
Implemented PEDIA integration into VarFish ticket varfish-org#399
eeb150c
@ahujameg
Merge branch 'varfish-org:main' into pedia
9ce7b40
@ahujameg
Implemented PEDIA integration into VarFish ticket varfish-org#399
f4a363a
@ahujameg
Merge branch 'pedia' of https://github.com/ahujameg/varfish-server in…
f0de558 
…to pedia
@ahujameg
Updating package-lock.json
013e012
@ahujameg
Updating package-lock.json
7bc5ba1
@ahujameg
Merge remote-tracking branch 'origin/pedia' into pedia
be070fe 
# Conflicts:
#	package-lock.json
@ahujameg
Updating package-lock.json
e8c322c
@ahujameg
Merge remote-tracking branch 'origin/pedia' into pedia
e0e38f4
@ahujameg
Merge branch 'varfish-org:main' into pedia
54da902
@ahujameg
Updating package-lock.json
8c3e135
@ahujameg
Merge branch 'pedia' of https://github.com/ahujameg/varfish-server in…
d65950d 
…to pedia
@ahujameg
Updating package-lock.json
beec4bf
@ahujameg
Merge remote-tracking branch 'origin/pedia' into pedia
2d4a396
@ahujameg
Updating package-lock.json
596c9d8
@ahujameg
Merge remote-tracking branch 'origin/pedia' into pedia
8607eb9
@ahujameg
Updating package-lock.json
5b90dce
@ahujameg
Updating package-lock.json
67f068a
@ahujameg
Updating package-lock.json
16fd695
@ahujameg
Updating package-lock.json
2132267
stolpeo
stolpeo requested changes Aug 8, 2024
View reviewed changes
backend/variants/file_export.py Outdated Show resolved Hide resolved
backend/variants/models/scores.py Show resolved Hide resolved
frontend/src/variants/components/FilterForm.vue Outdated Show resolved Hide resolved
frontend/src/variants/components/FilterForm/PrioritizationPane.vue Outdated Show resolved Hide resolved
frontend/src/variants/components/FilterForm/PrioritizationPane.vue Outdated Show resolved Hide resolved
package-lock.json Outdated Show resolved Hide resolved
ahujameg and others added 13 commits September 23, 2024 13:42
@ahujameg
Implement review comments.
d316793
@ahujameg
Merge branch 'pedia' of https://github.com/ahujameg/varfish-server in…
6641237 
…to pedia
@ahujameg
Merge branch 'main' into pedia
2998243
@ahujameg
Implement review comments.
718c86c
@ahujameg
Merge branch 'pedia' of https://github.com/ahujameg/varfish-server in…
6c814a4 
…to pedia
@ahujameg
Implement review comments.
30ece82
@ahujameg
Merge branch 'pedia' of https://github.com/ahujameg/varfish-server in…
17e1cad 
…to pedia
@ahujameg
Implementing review comments
13ec8ad
@ahujameg
Merge branch 'pedia' of https://github.com/ahujameg/varfish-server in…
4faf141 
…to pedia
@ahujameg
Implementing review comments
6bcc360
@ahujameg
Merge branch 'pedia' of https://github.com/ahujameg/varfish-server in…
b6ba13b 
…to pedia
@ahujameg
Implementing review comments
6ba33e6
@ahujameg
Merge branch 'pedia' of https://github.com/ahujameg/varfish-server in…
1680773 
…to pedia
@stolpeo stolpeo marked this pull request as ready for review September 26, 2024 10:51
@stolpeo stolpeo self-requested a review September 26, 2024 10:55
stolpeo
stolpeo requested changes Sep 26, 2024
View reviewed changes
Copy link
Contributor

@stolpeo stolpeo left a comment

Choose a reason for hiding this comment

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

Hello @ahujameg looking good to me, please only solve the issue with the base.py and LDAP parameters. Also, please run lint on the frontend code as this fails. Also the docker build fails, that needs to be fixed as well.

backend/config/settings/base.py Outdated Show resolved Hide resolved
@stolpeo
Copy link
Contributor

stolpeo commented Sep 26, 2024

I noticed that lint fails for the reev-frontend-lib. Did you do a rebase on the main branch @ahujameg ?

@stolpeo
Copy link
Contributor

stolpeo commented Sep 26, 2024

@ahujameg ok the docker build fail seems to be an issue with permissions. you can ignore that for now.

@ahujameg
Copy link
Collaborator

I noticed that lint fails for the reev-frontend-lib. Did you do a rebase on the main branch @ahujameg ?

yes, I took the latest code. The same error is observed in other PR as well so I thought its not related to my changes, for example>
https://github.com/varfish-org/varfish-server/actions/runs/11007235045/job/30562802496?pr=2002

stolpeo
stolpeo approved these changes Sep 27, 2024
View reviewed changes
Copy link
Contributor

@stolpeo stolpeo left a comment

Choose a reason for hiding this comment

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

LGTM

@stolpeo stolpeo merged commit 6f695ec into varfish-org:main Sep 27, 2024
6 of 8 checks passed
@varfish-bot varfish-bot mentioned this pull request Sep 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahujameg ahujameg ahujameg left review comments

@stolpeo stolpeo stolpeo approved these changes

Assignees

@ahujameg ahujameg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@holtgrewe @ahujameg @stolpeo