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

[PL] Add basic weather sentences #1993

Merged
merged 7 commits into from
Mar 4, 2024

Conversation

deejay1
Copy link
Contributor

@deejay1 deejay1 commented Feb 24, 2024

Add basic support for fetching weather

responses/pl/HassGetWeather.yaml Outdated Show resolved Hide resolved
responses/pl/HassGetWeather.yaml Outdated Show resolved Hide resolved
@home-assistant
Copy link

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 February 26, 2024 07:45
@Uriziel01
Copy link
Contributor

@witold-gren I think the weather responses here are more polished than in #1996, I think we should merge this and just overwrite some other changes where it makes sense.

@deejay1
Copy link
Contributor Author

deejay1 commented Feb 26, 2024

@witold-gren I think the weather responses here are more polished than in #1996, I think we should merge this and just overwrite some other changes where it makes sense.

@Uriziel01 I could do the weather upgrades from #1996 in here, WDYT?

@Uriziel01
Copy link
Contributor

@witold-gren I think the weather responses here are more polished than in #1996, I think we should merge this and just overwrite some other changes where it makes sense.

@Uriziel01 I could do the weather upgrades from #1996 in here, WDYT?

I would do it the other way around and migrate changes from here into #1996 as that PR is absolutely enormous and I would like to avoid conflicts as much as possible just to be safe.

@deejay1 deejay1 marked this pull request as ready for review February 26, 2024 09:25
@deejay1
Copy link
Contributor Author

deejay1 commented Feb 26, 2024

@Uriziel01 sure, review comments fixed, waiting for workflow approvals :/

Copy link
Contributor

@witold-gren witold-gren left a comment

Choose a reason for hiding this comment

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

good update 💪🏻 I only have a request to make some cosmetic changes

sentences/pl/weather_HassGetWeather.yaml Outdated Show resolved Hide resolved
response: 8 °C i pada deszcz

- sentences:
- "jaka jest pogoda w Warszawa"
Copy link
Contributor

Choose a reason for hiding this comment

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

The correct sentence should be "jaka jest pogoda w Warszawie"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure you can do automatic declination of entities. This could work with aliases though, but by using default entity names here you match the UI

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you're right but the future these declensions will be used to create sentences for the ML model. I would prefer it to sound correctly in Polish 😀 check this issue #976

tests/pl/weather_HassGetWeather.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@jlpouffier jlpouffier left a comment

Choose a reason for hiding this comment

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

Approved automagically, as I am not speaking Polish
I do not see anything out of the ordinary structure-wise.

@deejay1
Copy link
Contributor Author

deejay1 commented Feb 28, 2024

@Uriziel01 can you reapprove it ?

@synesthesiam synesthesiam dismissed Uriziel01’s stale review March 4, 2024 18:25

Changes have been made

@synesthesiam synesthesiam merged commit cdd28d9 into home-assistant:main Mar 4, 2024
2 checks passed
schizza pushed a commit to schizza/intents that referenced this pull request Mar 16, 2024
* [PL] Add basic weather sentences

* [PL] Minor fixes in wording after review

* [PL] Review fixes
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.

6 participants