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

refactor: rust query engine #193

Merged
merged 4 commits into from
Oct 10, 2024
Merged

refactor: rust query engine #193

merged 4 commits into from
Oct 10, 2024

Conversation

JabobKrauskopf
Copy link
Member

@JabobKrauskopf JabobKrauskopf commented Sep 4, 2024 •
edited
Loading

reimplemented the rust part of the query engine

Every single action (and therefore also tests) is supposed to fail as building the python package cannot work before the bindings issue is completed in #172

Linting and formatting of the rust part succeeds and can be checked by running cargo clippy --workspace --all-targets --all-features --exclude py-medmodels -- -D warnings and cargo fmt -- --check locally

Tests can be checked using cargo test --workspace --exclude py-medmodels

@JabobKrauskopf JabobKrauskopf force-pushed the 171-rewrite-query-engine-in-rust branch 2 times, most recently from b220f55 to 6bf734c Compare September 10, 2024 14:09
@JabobKrauskopf JabobKrauskopf changed the title refactor: query engine refactor: rust query engine Sep 10, 2024
@JabobKrauskopf JabobKrauskopf force-pushed the 171-rewrite-query-engine-in-rust branch 13 times, most recently from f6e23da to 5df1278 Compare September 17, 2024 07:52
@JabobKrauskopf JabobKrauskopf force-pushed the 171-rewrite-query-engine-in-rust branch 14 times, most recently from 257d69a to 41413df Compare September 19, 2024 21:43
@JabobKrauskopf JabobKrauskopf requested a review from a team as a code owner October 9, 2024 00:45
@JabobKrauskopf JabobKrauskopf marked this pull request as draft October 9, 2024 00:53
@JabobKrauskopf JabobKrauskopf force-pushed the 171-rewrite-query-engine-in-rust branch from 1ab4034 to da35602 Compare October 9, 2024 11:10
@JabobKrauskopf JabobKrauskopf force-pushed the 171-rewrite-query-engine-in-rust branch 2 times, most recently from 4a20f2d to c4995f5 Compare October 9, 2024 11:15
@JabobKrauskopf JabobKrauskopf marked this pull request as ready for review October 9, 2024 11:15
@JabobKrauskopf JabobKrauskopf force-pushed the 171-rewrite-query-engine-in-rust branch from c4995f5 to ec2070a Compare October 9, 2024 11:17
@JabobKrauskopf
Copy link
Member Author

There are still some TODO comments in the code. Those are supposed to be there and will be fixed after my vacation. See #165

MarIniOnz
MarIniOnz previously approved these changes Oct 9, 2024
View reviewed changes
Copy link
Contributor

@MarIniOnz MarIniOnz left a comment

Choose a reason for hiding this comment

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

Could not be of much help, but did not find any clear typo (did not do much more than skimming to be honest).

crates/medmodels-core/src/medrecord/graph/mod.rs Show resolved Hide resolved
crates/medmodels-core/src/medrecord/querying/edges/operand.rs Show resolved Hide resolved
LauraBoenchenLB
LauraBoenchenLB reviewed Oct 9, 2024
View reviewed changes
Copy link
Contributor

@LauraBoenchenLB LauraBoenchenLB left a comment

Choose a reason for hiding this comment

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

A few comments, more questions than anything else :)

crates/medmodels-core/src/medrecord/datatypes/attribute.rs Show resolved Hide resolved
crates/medmodels-core/src/medrecord/datatypes/attribute.rs Show resolved Hide resolved
crates/medmodels-core/src/medrecord/datatypes/attribute.rs Outdated Show resolved Hide resolved
crates/medmodels-core/src/medrecord/datatypes/mod.rs Show resolved Hide resolved
crates/medmodels-core/src/medrecord/querying/values/operand.rs Show resolved Hide resolved
@JabobKrauskopf JabobKrauskopf force-pushed the 171-rewrite-query-engine-in-rust branch from ec2070a to c44619c Compare October 9, 2024 18:52
LauraBoenchenLB
LauraBoenchenLB approved these changes Oct 10, 2024
View reviewed changes
Copy link
Contributor

@LauraBoenchenLB LauraBoenchenLB left a comment

Choose a reason for hiding this comment

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

Great job!! SO EXCITED TO TRY THIS OUT 🦭

MarIniOnz
MarIniOnz approved these changes Oct 10, 2024
View reviewed changes
Copy link
Contributor

@MarIniOnz MarIniOnz left a comment

Choose a reason for hiding this comment

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

Rusty seal for you!
DALLE2024-10-1010 45 57-Acutesealworkingonanenginegivingathumbs-uptoacamerabuttheentirescenelooksrustyandwornout Thesealandtheengineappearrusty-ezgif com-webp-to-jpg-converter

@JabobKrauskopf JabobKrauskopf merged commit ef09e2e into epic/165-query-engine-refactor-1 Oct 10, 2024
0 of 6 checks passed
@JabobKrauskopf JabobKrauskopf deleted the 171-rewrite-query-engine-in-rust branch October 10, 2024 09:12
JabobKrauskopf added a commit that referenced this pull request Oct 11, 2024
@JabobKrauskopf
refactor: rust query engine (#193)
d2c7120
JabobKrauskopf added a commit that referenced this pull request Oct 11, 2024
@JabobKrauskopf
refactor: rust query engine (#193)
6181297
@JabobKrauskopf JabobKrauskopf mentioned this pull request Oct 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarIniOnz MarIniOnz MarIniOnz approved these changes

@LauraBoenchenLB LauraBoenchenLB LauraBoenchenLB approved these changes

@OFranke OFranke Awaiting requested review from OFranke OFranke was automatically assigned from limebit/medmodels-software-engineering

@FloLimebit FloLimebit Awaiting requested review from FloLimebit FloLimebit was automatically assigned from limebit/medmodels-software-engineering

@philippgrosser philippgrosser Awaiting requested review from philippgrosser philippgrosser was automatically assigned from limebit/medmodels-data-science-team

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rewrite Query Engine in Rust
3 participants
@JabobKrauskopf @MarIniOnz @LauraBoenchenLB