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

Remove uses of cloneDeep in qedge2bteedge #52

Merged
merged 2 commits into from
Oct 22, 2021
Merged

Remove uses of cloneDeep in qedge2bteedge #52

merged 2 commits into from
Oct 22, 2021

Conversation

tokebe
Copy link
Member

@tokebe tokebe commented Oct 19, 2021

(addresses biothings/biothings_explorer#323)

This PR removes uses of cloneDeep. Edges which were previously cloned are not used or edited in any way after being converted. Some qedges are converted to multiple BTEEdges with different parameters added in the process, meaning that new = { ...original } assignment with references should suffice. Testing several queries, including one and two hop, etc, shows no change in outputs.

@tokebe tokebe changed the title Rm q2b clone Remove uses of cloneDeep in qedge2bteedge Oct 19, 2021
@tokebe
Copy link
Member Author

tokebe commented Oct 19, 2021

It appears the actual tests are succeeding, there is some problem with the coverage report causing checks to fail.

@colleenXu
Copy link
Contributor

@newgene I recommend this for merge / push to prod. ASAP is okay if needed for testing/fixing other issues.

I've tried out a few queries and the behavior is as-expected. the performance (speed) seems comparable as well.

@newgene newgene merged commit f553049 into main Oct 22, 2021
@newgene newgene deleted the rm_q2b_clone branch October 22, 2021 21:20
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.

3 participants