UUDigitalHumanitieslab / parseport

Dutch sentence parser for Spindle + Æthel (and maybe others in the future).
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Feature/aethel search #28

Closed XanderVertegaal closed 2 months ago

XanderVertegaal commented 2 months ago

This PR does a couple of things. In hindsight, I should have split them up, but I hope they are still manageable.

Features:

I also added 'ProofPipe' to transform the incoming types from the backend to user-expected format (addresses #27 ). The transformation rules are pretty basic and I have only added a few unit tests. I can imagine we will have to adapt it as we come across cases that are not captured appropriately. If you think we should rather handle this in the backend, that would also be fine with me.

bbonf commented 2 months ago

A general comment: I think every one of your bullet points could be its own PR (except for maybe combining the third and fourth). That would be my personal preference, smaller PRs for independent changes

XanderVertegaal commented 2 months ago

Thanks for your review! I agree this would have been better as a set of smaller PRs, and I will do my best to avoid having PRs that try to do too many things at once, like this one. My apologies!

XanderVertegaal commented 2 months ago

I've got all of your comments. Hopefully you can run this branch now! You will need the Aethel pickle. You will need to download it from the Aethel repo and put it in backend/aethel_db/data/.

There are two things I'd like your feedback on:

BTW, for easy testing, you do not need to run this application in Docker containers. Running the frontend and backend locally should work as well.

XanderVertegaal commented 2 months ago

Ready for review again! I could not test the prod build locally, because my VM keeps running out of memory trying to load the full Aethel dataset. This may not be an issue on the server, but something we should test (on test/acc) before deploying.