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 broken MultiLine chart #9497

Closed
wants to merge 3 commits into from

Conversation

willbarrett
Copy link
Member

@willbarrett willbarrett commented Apr 9, 2020

CATEGORY

Choose one

  • Bug Fix
  • Enhancement (new features, refinement)
  • Refactor
  • Add tests
  • Build / Development Environment
  • Documentation

SUMMARY

The lines altered in this PR were introduced 8 days ago in this commit: f0f4f7e

These lines broke

the Multi Line visualization, as that visualization has no from_dttm or to_dttm attributes. MultiLine visualizations on current master display the error 'MultiLineViz object has no attribute 'from_dttm'.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:
Screen Shot 2020-04-09 at 12 11 20 PM
After:
Screen Shot 2020-04-09 at 12 45 32 PM

TEST PLAN

I've manually tested the change locally. You can verify the fix by navigating to:
Dashbaoards -> Misc Charts
and scrolling to the bottom where the "Multi Line" chart is displayed.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

REVIEWERS

@villebro @dpgaspar @ktmud

Copy link
Member

@ktmud ktmud left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the fix!

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing! 👍

@willbarrett
Copy link
Member Author

Travis has passed CI but the status wasn't reported to Github :(

@villebro
Copy link
Member

Travis is currently almost doing more harm than good.

@willbarrett
Copy link
Member Author

Agreed. We're going to try to support the Github Actions work next week.

@willbarrett
Copy link
Member Author

OK, build passed again, still no status update from Travis. Does anyone have admin access to merge in this type of situation?

@villebro
Copy link
Member

It seems once GH drops Travis like this, there's no coming back. It's not elegant I know, but I propose closing this and opening a new PR, as this is a pretty critical fix.

@willbarrett
Copy link
Member Author

OK, closing + reopening.

@willbarrett willbarrett deleted the wbarrett/viz-py-fix branch April 14, 2020 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants