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

Fix handling of empty results from Rejseplanen #25610

Merged

Conversation

@DarkFox
Copy link
Contributor

commented Jul 31, 2019

Breaking Change:

When there are no upcoming departures, the sensor now returns state unknown, and removes attributes instead of setting them to n/a.

Any existing templates looking for the n/a state should be updated to look for unknown instead, and all templates should be altered to handle potentially missing attributes.

Description:

Remove "n/a" attributes, and return None for state, when no upcoming departures are returned from API.

Related issue (if applicable): fixes #25566

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

Dev automation moved this from Needs review to Review in progress Jul 31, 2019

@DarkFox DarkFox force-pushed the DarkFox:fix-rejseplanen-empty-result branch from eca7029 to be53a4f Jul 31, 2019

@DarkFox

This comment has been minimized.

Copy link
Contributor Author

commented Jul 31, 2019

I'm guessing I'll need to rebase again once black is merged, to fix that linting error.

@MartinHjelmare
Copy link
Member

left a comment

Looks good!

Dev automation moved this from Review in progress to Reviewer approved Jul 31, 2019

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

There's a lint error merged in dev branch at the moment. That's why the build is failing.

@DarkFox DarkFox force-pushed the DarkFox:fix-rejseplanen-empty-result branch from d82c4e6 to 7e3c24e Aug 1, 2019

@DarkFox

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2019

@MartinHjelmare Rebased and reformatted with Black, should be ready for merging. :)

@MartinHjelmare MartinHjelmare changed the title Fix handling of empty results from Rejseplanen (Fixes #25566) Fix handling of empty results from Rejseplanen Aug 1, 2019

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

We should label it a breaking change, right?

@DarkFox

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2019

Technically yes, it changes the output.

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

Please add a breaking change paragraph in the PR description and write what has changed and what the user needs to do to cope with the breaking change.

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

Ping me when it's ready and I'll merge.

@DarkFox

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2019

@MartinHjelmare Right, that should be it. Breaking change paragraph added. 👍

@MartinHjelmare MartinHjelmare merged commit c3cdd3e into home-assistant:dev Aug 1, 2019

11 checks passed

CI Build #20190801.96 succeeded
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (FullCheck Pylint) FullCheck Pylint succeeded
Details
CI (Overview CheckFormat) Overview CheckFormat succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
CI (Tests PyTest Python36) Tests PyTest Python36 succeeded
Details
CI (Tests PyTest Python37) Tests PyTest Python37 succeeded
Details
cla-bot Everyone involved has signed the CLA
codecov/patch Coverage not affected when comparing c2556d9...7e3c24e
Details
codecov/project 94% (target 90%)
Details

Dev automation moved this from Reviewer approved to Done Aug 1, 2019

@MartinHjelmare MartinHjelmare added this to the 0.97.0 milestone Aug 1, 2019

balloob added a commit that referenced this pull request Aug 1, 2019

Fix handling of empty results from Rejseplanen (#25610)
* Improve handling of empty results from Rejseplanen (Fixes #25566)

* Exclude attributes with null value

* Add period back into docstring

* Fix formatting

@lock lock bot locked and limited conversation to collaborators Aug 2, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
4 participants
You can’t perform that action at this time.