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

Map and Timeseries Session #2

Merged
merged 3 commits into from
Jun 4, 2021
Merged

Map and Timeseries Session #2

merged 3 commits into from
Jun 4, 2021

Conversation

Cadair
Copy link
Member

@Cadair Cadair commented Jun 2, 2021

@dstansby
Copy link
Member

dstansby commented Jun 2, 2021

Suggestions:

  • Link to the GenericMap docstring when you mention map properties
  • The 'go' etc. matplotlib formatting strings are a really un-intuitive way of specifying formatting, spell them out instead (e.g. color='green', marker='o')
  • Label and add a legend to the plotted coordinates so it's easy to see which one is which
  • Us a colormap that has an obvious zero point colour for plotting hmi (e.g. RdBu_r)
plt.figure(figsize=(10,10))
plt.subplot(projection=aia171)
aia171.plot(clip_interval=[1, 99.9]*u.percent)

lines = hmilos.draw_contours(levels, cmap="seismic")
plt.colorbar(lines, ticks=levels, label=u.Gauss)

I'm really confused here. Are you implicitly using the current axes to plot the contours? Wherever possible the axes being used to plot should be explicitly assigned a variable and passed to the various plotting functions, to make it clear what is being plotted on what.

@Cadair
Copy link
Member Author

Cadair commented Jun 2, 2021

Us a colormap that has an obvious zero point colour for plotting hmi (e.g. RdBu_r)

I actually chose this for exactly this reason, it doesn't have a large gradient around the zero. All the other ones ended up with most of the contours being grey. I thought this was easier than changing the limits or the stretch etc.

@Cadair
Copy link
Member Author

Cadair commented Jun 2, 2021

Label and add a legend to the plotted coordinates so it's easy to see which one is which

I would rather talk through this than add even more clutter to the already very verbose lines.

@ayshih ayshih marked this pull request as ready for review June 4, 2021 02:05
@ayshih ayshih merged commit 4b84580 into sunpy:main Jun 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants