monarch-initiative / semsimian

Simple rust implementation of semantic similarity
BSD 3-Clause "New" or "Revised" License
8 stars 5 forks source link

full_search mutable vs immutable self #112

Closed glass-ships closed 7 months ago

glass-ships commented 11 months ago

currently the full_search function requires a mutable self, but (for reasons admittedly beyond my understanding), the semsimian server needs it to be immutable.

is this a reasonable/feasible refactor?

monicacecilia commented 10 months ago

Dear @justaddcoffee, @hrshdhgd, and @caufieldjh, Could I kindly ask you to please help us address this issue? More background at https://github.com/monarch-initiative/monarch-app/issues/233

We would really like to address this for our December 14 Milestone. Let's please connect on updates during the next Data Call on December 7th. Your help is greatly appreciated. 🌷

@sagehrke @cmungall 👀 👆

hrshdhgd commented 10 months ago

Could you direct us as to where in the semsimian-server code does the object need to be immutable?

hrshdhgd commented 10 months ago

If it is utils.rs , this may be a potential solution? I'm not sure it'll work but worth a try. The refactor within semsimian itself will have a domino effect on other parts of the project and impacts the caching design which may need significant rewrite and hard to put a timeline on it since we are all rust novices.

glass-ships commented 10 months ago

thanks harshad! i'll give this a test sometime this week, i'm not super sure about the implementation of search but hopefully the PR you propose will suffice for our case

caufieldjh commented 10 months ago

I would also favor https://github.com/monarch-initiative/semsimian-server/pull/9 if it works as expected

justaddcoffee commented 7 months ago

@hrshdhgd is this ticket closed by https://github.com/monarch-initiative/semsimian-server/pull/9?

hrshdhgd commented 7 months ago

I think so...