Skip to content

Fix TimeSeries examples not properly displayed in UI#2064

Merged
abidlabs merged 10 commits into
mainfrom
dawood/timeseries-examples
Aug 27, 2022
Merged

Fix TimeSeries examples not properly displayed in UI#2064
abidlabs merged 10 commits into
mainfrom
dawood/timeseries-examples

Conversation

@dawoodkhan82
Copy link
Copy Markdown
Collaborator

@dawoodkhan82 dawoodkhan82 commented Aug 23, 2022

Description

Fix TimeSeries Example

Screen.Recording.2022-08-23.at.12.44.42.PM.mov

Please include:

  • relevant motivation
  • a summary of the change
  • which issue is fixed.
  • any additional dependencies that are required for this change.

Closes: #1767

Checklist:

  • I have performed a self-review of my own code
  • My code follows the style guidelines of this project
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@dawoodkhan82 dawoodkhan82 linked an issue Aug 23, 2022 that may be closed by this pull request
1 task
@github-actions
Copy link
Copy Markdown
Contributor

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-2064-all-demos

@dawoodkhan82
Copy link
Copy Markdown
Collaborator Author

dawoodkhan82 commented Aug 23, 2022

Theres an issue with cache_examples. Taking a look now.

Comment thread gradio/components.py
return None
if isinstance(y, str):
y = pd.read_csv(y)
return {"headers": y.columns.values.tolist(), "data": y.values.tolist()}
Copy link
Copy Markdown
Member

@pngwn pngwn Aug 23, 2022

Choose a reason for hiding this comment

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

All return values need to have headers now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh maybe this is only true for Dataframe, ideally we'd make the interfaces consistent since they are very similar. What is the reason for this change, how does it address the issue?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Removed this change, and instead changed the frontend to convert the timeseries obj to string (to fix examples).

@dawoodkhan82 dawoodkhan82 requested a review from pngwn August 24, 2022 03:11
Copy link
Copy Markdown
Collaborator

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @dawoodkhan82 ! Confirm it works locally for me.

Code change makes sense to me but defer to you since you are more familiar with it!

reader.readAsText(blob);
}

function dict_to_string(dict: any) {
Copy link
Copy Markdown
Collaborator Author

@dawoodkhan82 dawoodkhan82 Aug 24, 2022

Choose a reason for hiding this comment

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

Convert the timeseries dict to string format to be accepted by Chart component.

pngwn
pngwn previously requested changes Aug 24, 2022
Comment thread ui/packages/app/src/components/TimeSeries/TimeSeries.svelte Outdated
Comment thread ui/packages/app/src/components/TimeSeries/TimeSeries.svelte Outdated
Comment thread ui/packages/app/src/components/TimeSeries/TimeSeries.svelte Outdated
@abidlabs
Copy link
Copy Markdown
Member

LGTM, works for me locally. It looks like @pngwn's comments were resolved, so let's go ahead and merge this @dawoodkhan82?

@abidlabs
Copy link
Copy Markdown
Member

abidlabs commented Aug 27, 2022

I'll go ahead and merge this in @dawoodkhan82 to avoid conflicts from popping up later!

@abidlabs abidlabs merged commit 43c7e8a into main Aug 27, 2022
@abidlabs abidlabs deleted the dawood/timeseries-examples branch August 27, 2022 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TimeSeries examples not properly displayed in UI

4 participants