Skip to content

Fix handling of empty results from Rejseplanen#25610

Merged
MartinHjelmare merged 4 commits into
home-assistant:devfrom
DarkFox:fix-rejseplanen-empty-result
Aug 1, 2019
Merged

Fix handling of empty results from Rejseplanen#25610
MartinHjelmare merged 4 commits into
home-assistant:devfrom
DarkFox:fix-rejseplanen-empty-result

Conversation

@DarkFox
Copy link
Copy Markdown
Contributor

@DarkFox DarkFox 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.

Comment thread homeassistant/components/rejseplanen/sensor.py Outdated
@DarkFox DarkFox force-pushed the fix-rejseplanen-empty-result branch from eca7029 to be53a4f Compare July 31, 2019 19:42
@DarkFox
Copy link
Copy Markdown
Contributor Author

DarkFox commented Jul 31, 2019

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

Comment thread homeassistant/components/rejseplanen/sensor.py Outdated
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Looks good!

@MartinHjelmare
Copy link
Copy Markdown
Member

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

@DarkFox DarkFox force-pushed the fix-rejseplanen-empty-result branch from d82c4e6 to 7e3c24e Compare August 1, 2019 16:43
@DarkFox
Copy link
Copy Markdown
Contributor Author

DarkFox 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
Copy link
Copy Markdown
Member

We should label it a breaking change, right?

@DarkFox
Copy link
Copy Markdown
Contributor Author

DarkFox commented Aug 1, 2019

Technically yes, it changes the output.

@MartinHjelmare
Copy link
Copy Markdown
Member

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
Copy link
Copy Markdown
Member

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

@DarkFox
Copy link
Copy Markdown
Contributor Author

DarkFox 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
@MartinHjelmare MartinHjelmare added this to the 0.97.0 milestone Aug 1, 2019
balloob pushed a commit that referenced this pull request Aug 1, 2019
* 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

None yet

Development

Successfully merging this pull request may close these issues.

Input warning for rejseplanen

4 participants