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

[NE] Added Initial Support for Nepali Language #2698

Merged
merged 9 commits into from
Dec 24, 2024

Conversation

Rrp13
Copy link
Contributor

@Rrp13 Rrp13 commented Dec 1, 2024

Updated _common.yaml, homeassistant_HassTurnOff and homeassistant_HassTurnOn

Copy link

@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

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

Hi @Rrp13

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@home-assistant
Copy link

home-assistant bot commented Dec 1, 2024

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@home-assistant home-assistant bot marked this pull request as draft December 1, 2024 17:02
@Rrp13 Rrp13 marked this pull request as ready for review December 1, 2024 17:05
@v1k70rk4
Copy link
Member

v1k70rk4 commented Dec 1, 2024

Hi, we're glad you're here ;)

To keep a PR manageable, we break the work into smaller parts. This way, it's clear what has already been completed and what still needs to be done.

I suggest including only the files you've already completed in the PR:
homeassistant_HassTurnOff / HassTurnOn
and the _fixtures.yaml / _common.yaml files.

@v1k70rk4
Copy link
Member

v1k70rk4 commented Dec 1, 2024

And one more observation:
The homeassistant_HassTurnOff/On is specifically for general cases.
In the tests, you mention two domains (light, fan) that already have separate parts specifically for this:

  • fan_HassTurnOff/On
  • light_HassTurnOff/On

It's worth starting from the English version and translating that. If you have any questions, feel free to ask!

@Rrp13 Rrp13 marked this pull request as draft December 1, 2024 20:23
@Rrp13
Copy link
Contributor Author

Rrp13 commented Dec 1, 2024

@v1k70rk4 Thank you for your suggestions. Based on your advice, I have included only the completed files. Regarding the test, I began by translating the English version and, as per your recommendation, I have removed the domain from the test.

@Rrp13 Rrp13 marked this pull request as ready for review December 1, 2024 20:35
@Rrp13 Rrp13 marked this pull request as draft December 1, 2024 20:37
@Rrp13 Rrp13 marked this pull request as ready for review December 1, 2024 20:38
@v1k70rk4
Copy link
Member

v1k70rk4 commented Dec 1, 2024

Until a maintainer approves it, I checked if it would pass the test, and unfortunately, it doesn’t.

=============================================== short test summary info ================================================  
FAILED tests/test_language_sentences.py::test_homeassistant_HassTurnOff[ne] - AssertionError: Recognition failed for 'बैठकको बत्ती निबाइदेउ न'  
FAILED tests/test_language_sentences.py::test_homeassistant_HassTurnOn[ne] - AssertionError: Recognition failed for 'बैठकको बत्ती बाल्देउ न'  
============================================ 2 failed, 101 passed in 11.88s ============================================  

This means that: The sentence "बैठकको बत्ती निबाइदेउ न" in its current form does not fit the given template because the verb type "निबाइदेउ" is not covered by the template. The template includes "निबाउनु", but "निबाइदेउ" is a conjugated, imperative form of that verb, which requests the action to be performed.

To make this fit, you would need to add the following variations to the template:

निबाइदेउ
निबाइदिनुस

This way, sentences like "बैठकको बत्ती निबाइदेउ न" and similar ones would match the pattern.

By the way, how did you get started with the translation? Windows/Linux/Visual Studio Core/Codespace? You can read about how to run tests on your own machine here. You’ll need it if you want to work on the Nepali intents :)

@Rrp13 Rrp13 marked this pull request as draft December 1, 2024 22:26
@Rrp13
Copy link
Contributor Author

Rrp13 commented Dec 1, 2024

@v1k70rk4 Thanks again for your input. I will look into it. The specific thing that you mentioned is explicitly defined, and it should have worked. Could you please let me know which test you did so that I can figure out the issue. I am using vscode in linux.
I did the language validation test, and it passed.

@v1k70rk4
Copy link
Member

v1k70rk4 commented Dec 1, 2024

If /script/test runs fine for you, then don’t worry :) My computer completely freaked out with the Nepali characters, so maybe the bug is on my end :) Just submit it as it is, and someone will review it soon.

@Rrp13 Rrp13 marked this pull request as ready for review December 1, 2024 23:02
@Rrp13 Rrp13 marked this pull request as draft December 1, 2024 23:13
@Rrp13
Copy link
Contributor Author

Rrp13 commented Dec 2, 2024

I have rechecked and perfomed all the tests mentioned in the documents and all of them have passed for HassTurnOn and HassTurnOff. @v1k70rk4 Thank you for helping me out.

@Rrp13 Rrp13 marked this pull request as ready for review December 2, 2024 00:46
@synesthesiam
Copy link
Contributor

Looks great, thank you!

@synesthesiam synesthesiam merged commit 5b24710 into home-assistant:main Dec 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants