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

Simplify translation of terms #12

Merged
merged 1 commit into from
Jan 25, 2023

Conversation

UnniKohonen
Copy link
Contributor

This PR simplifies translation of suggested terms by using the language parameter of Annif API's suggest method. Calls to Finto API are removed.

Implements #10

@UnniKohonen UnniKohonen added the enhancement New feature or request label Dec 30, 2022
Copy link
Member

@osma osma left a comment

Choose a reason for hiding this comment

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

Looks very good - we will get rid of a lot of code - and seems to work well.

I'm not sure what is the best merging strategy here (should we merge directly to main or to some other branch? @juhoinkinen ?) but once we figure that out, this can go in.

@juhoinkinen
Copy link
Member

Hmm, I think it's best to merge to update-2022, because that branch is deployed for ai.dev.finto.fi , and main branch for ai.finto.fi.

@osma osma changed the base branch from main to update-2022 January 23, 2023 15:26
@osma
Copy link
Member

osma commented Jan 23, 2023

I changed the base branch, but now we have many more commits in this PR. I think there should be a merge from main to update-2022 first that would take care of the commits that are not yet in update-2022.

@juhoinkinen
Copy link
Member

I changed the base branch, but now we have many more commits in this PR. I think there should be a merge from main to update-2022 first that would take care of the commits that are not yet in update-2022.

Ok, I'll merge on my laptop and push.

@juhoinkinen juhoinkinen changed the base branch from update-2022 to main January 23, 2023 15:43
@juhoinkinen juhoinkinen changed the base branch from main to update-2022 January 23, 2023 15:43
@juhoinkinen
Copy link
Member

To make "the extra commits" disappear from thia PR, I also had to switch the base branch to main (any other branch could have done) and back to update-2022. This was suggested in StackOverflow.

Copy link
Member

@osma osma left a comment

Choose a reason for hiding this comment

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

Can be merged now IMHO

@UnniKohonen UnniKohonen marked this pull request as ready for review January 25, 2023 11:15
@UnniKohonen UnniKohonen merged commit d9e9aea into update-2022 Jan 25, 2023
@UnniKohonen UnniKohonen deleted the issue10-simplify-translation branch January 25, 2023 11:17
@osma
Copy link
Member

osma commented Jan 25, 2023

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants