Skip to content
This repository has been archived by the owner on May 6, 2024. It is now read-only.

Update plotting script to include xkey and ykey #259

Merged
merged 3 commits into from
Sep 22, 2021

Conversation

dmadeka
Copy link
Contributor

@dmadeka dmadeka commented Sep 21, 2021

@heiner Solves #257

Taken from here

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 21, 2021
@dmadeka
Copy link
Contributor Author

dmadeka commented Sep 21, 2021

Not sure whats happening with flake8 here:

In [1]: a = "float,float. range of y values to plot. overrides automatic zoom for y axis."                                                                                                                         

In [2]: len(a)                                                                                                                                                                                                     
Out[2]: 76

@heiner
Copy link
Contributor

heiner commented Sep 21, 2021

Line 37 I believe.

@dmadeka
Copy link
Contributor Author

dmadeka commented Sep 21, 2021

Strange, running flake8 and black didnt show either error. Fixed.

@dmadeka
Copy link
Contributor Author

dmadeka commented Sep 21, 2021

unclear the failing tests are related to plot.py?

@heiner
Copy link
Contributor

heiner commented Sep 21, 2021

Yes this is obviously a flake (although I'd like to know what the underlying issue is, there's no way to get at that now).

I'll review and merge tomorrow.

@dmadeka
Copy link
Contributor Author

dmadeka commented Sep 21, 2021

Ill try look into it on my mac tonight -

Looks like the quit isnt really working

Copy link
Contributor

@heiner heiner left a comment

Choose a reason for hiding this comment

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

Generally looks good, three small comments below.

Copy link
Contributor

@heiner heiner 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, thanks!

@heiner heiner merged commit b69d4a1 into facebookresearch:main Sep 22, 2021
@dmadeka dmadeka deleted the updateplot branch September 22, 2021 16:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants