Fixed pyICU capabilities for emoji functionalities #407
Conversation
Thank you for the pull request!The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :) If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Data rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you! Maintainer checklist |
|
CC: @mhmohona can you please review the pr if there is no problem? |
|
Would you be able to fix the merge conflict here, @axif0? :) |
| # Check if PyICU is already installed using pkg_resources | ||
| installed_packages = {pkg.key for pkg in pkg_resources.working_set} | ||
| if package_name.lower() not in installed_packages: | ||
| print(f"{package_name} not found. Installing...") |
There was a problem hiding this comment.
As a user I would be alarmed if suddenly a package gets auto installed. I think it would be nice to describe in one sentence why installing PyICU is needed. WHat do you think @andrewtavis?
There was a problem hiding this comment.
I ran this branch in both Ubuntu and Windows, and tested by creating new environment, and it runs exactly like @andrewtavis gave the requirement to follow! Much appreciated @axif0. This PR is gonna be life saver.
However, the changes in get.py, process_uicode.py, and query_data.py file - it is showing the whole file has been been changed, but when I went through the code, i found not whole not the whole file, rather partial part of it's been changed. It would be easier to track what changes has been made if the previous format was kept.
What do you think @andrewtavis?
|
@mhmohona I dunno ether, but accidentally I worked on anther branch, and had to copy the file and paste this branch. Sorry for the inconvenient. And thank you for the testing.. |
|
Let's maybe open a new version of this that has just the needed changes to make this work given changes here and in #406 :) |
Contributor checklist
Description
Fetched and install correct version pyICU from here.
scribe-data get --lang german -dt emoji_keywordsBy calling the command or if we call
emoji-keywordsfrominteractive modewe check if pyICU installed or not. If installed then we skip installation process and if not install then the installation process will begun.We can add a question like
We need to install pyICU for emoji, As not installed do you want to install it?like that. Or we can call it automatically fromrequirement.txt(probably)From my side Works both ubuntu & windows perfectly. 😅
My hypothesis->
Well, when we want to install pyICU from pip it gets only compatible version for ubuntu. As pyICU has different releases, it doesn't detect first which releases it needs to install. like arm, amd or cp310/311/312 or else. So by calling the api and matching our architecture like windows, we get our releases. And then pyICU doesn't cause any error.