[time_series_with_matrices] Address feedbacks #534
Conversation
✅ Deploy Preview for taupe-gaufre-c4e660 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -49,7 +49,9 @@ We will use the following imports: | |||
| import numpy as np | |||
| import matplotlib.pyplot as plt | |||
| from matplotlib import cm | |||
| plt.rcParams["figure.figsize"] = (11, 5) #set default figure size | |||
| plt.rcParams["figure.figsize"] = (11, 5) # Set default figure size | |||
There was a problem hiding this comment.
@HumphreyYang can we remove this?
There was a problem hiding this comment.
Hi @mmcky,
I just experimented with deleting it, but it squeezed the time-series plot too much.
I think we might want to keep it.
There was a problem hiding this comment.
roger that @HumphreyYang can we then please make the size specific to the figure in question (as a special case).
There was a problem hiding this comment.
I think most of the graphs below are time series as it is the theme of the lecture, but I will check if it can be done.
There was a problem hiding this comment.
Hi @mmcky,
I think the graphs below are all time-series plots that I think would be best presented in the current ratio. Perhaps this is why it is not removed in our previous iteration : )
There was a problem hiding this comment.
@HumphreyYang roger that. I wonder if there is some way to indicate that in the comment so we don't keep having this conversation :-)
There was a problem hiding this comment.
Perhaps we can # Set default figure size # above the line so we know this is final decision : )
There was a problem hiding this comment.
Thanks @HumphreyYang lets change the comment to # custom figsize required for this lecture
|
|
|
Many thanks @mmcky, I think it is ready now. |
|
thanks @HumphreyYang |
This PR addresses most of comments in #461.