NDCubeSequence plotting mixin API. #98

DanRyanIrish · 2018-03-21T15:17:38Z

This PR introduces a new API for an NDCubeSequence plotting that will provide a more comprehensive plotting functionality for NDCubeSequence and more consistent API. This API/functionality should be mirrored NDCube where appropriate and vice versa.

This new mixin API will also allow a new NDCubeSequenceBase class independent of visualization and also of sunpy.

pep8speaks · 2018-03-21T15:17:40Z

Hello @DanRyanIrish! Thanks for updating the PR.

In the file ndcube/tests/test_sequence_plotting.py, following are the PEP8 issues :

Line 363:5: E265 block comment should start with '# '
Line 256:5: E265 block comment should start with '# '

Comment last updated on March 26, 2018 at 18:43 Hours UTC

…rg API changes.

…th placeholder errors.

DanRyanIrish · 2018-03-22T20:08:47Z

@Cadair, I think this is ready for a serious discussion now.

As an explanation, the cubesequence arg is there just so I can easily play with it without actually mixing in the class. All occurrences will be removed and replaced with self before merging.

…ke dimension states. Also got NDCubeSequencePlotMixin.plot working with LineAnimatorNDCubeSequence class for 2D sequences.

…cases where sequence dimensions > 2.

DanRyanIrish · 2018-03-22T21:45:08Z

I think I might have done another Danny's-1st-PR-style move here. +1,198 lines, -3 lines...woops!

DanRyanIrish · 2018-03-23T13:25:36Z

ndcube/mixins/sequence_plotting.py

+
+        axes_units: length 2 `list` of `astropy.unit.Unit` or valid unit `str` or None.
+            Denotes unit in which X-axis and Y-axis, respectively, should be displayed.
+            Only used if corresponding entry in axes_coordinates is a Quantity.


Error in this kwarg description. Should be same length as number of dimensions.

However, should probably also accept length 2 for images and length one for line plots so slider units don't have to be changed every time for high dimension sequences.

DanRyanIrish · 2018-03-23T13:27:47Z

ndcube/mixins/sequence_plotting.py

+            If None coordinates derived from the WCS objects will be used for all axes.
+            If a list, it should contain one element for each axis.  Each element should
+            be either an `astropy.units.Quantity` or a `numpy.ndarray` of coordinates for
+            each pixel, or a `str` denoting a valid extra coordinate.


Error in this kwarg description. Elements can also be None.

axes_coordinates should probably also accept length 2 for images and length one for line plots so slider units don't have to be changed every time for high dimension sequences.

Cadair · 2018-03-23T13:19:08Z

ndcube/mixins/sequence_plotting.py

+            If None coordinates derived from the WCS objects will be used for all axes.
+            If a list, it should contain one element for each axis.  Each element should
+            be either an `astropy.units.Quantity` or a `numpy.ndarray` of coordinates for
+            each pixel, or a `str` denoting a valid extra coordinate.


add None here

DanRyanIrish · 2018-03-23T13:31:05Z

ndcube/mixins/sequence_plotting.py

+
+        data_unit: `astropy.unit.Unit` or valid unit `str` or None
+            Unit in which data in a 2D image or animation should be displayed.  Only used if
+            visualization is a 2D image or animation, i.e. if image_axis has length 2.


Correct this description. Data unit also is applied to y-axis of line plot.

DanRyanIrish · 2018-03-23T13:46:34Z

Move the plot_as_cube options into separate plot_as_cube top level method.

DanRyanIrish · 2018-03-23T13:51:57Z

Move some helper functions into class. Those that use the object itself.

…hod. Also updaed docstrings of plot and plot_as_cube to reflect the what the API will become over future commits.

…PlotMixin.plot_as_cube.

DanRyanIrish · 2018-03-23T15:53:55Z

Wrap dimension-specific prep operations in plot into dimension-specific helper functions to clean up plot.

…plot and and plot_as_cube methods to external function. Also added more comments.

…ll stop bugs when dimensions are of same length. Also made NDCubeSequencePlotMixin a proper class by replacing the cubesequence input arg to self.

…f occurences.

… plot options. Also implement mixin in ndcubesequence module creating a non-plotting Base class and an NDCubeSequence class using plot mixin.

…ues were being reassigned instead of the axis unit. Also added some error checks and removed a few lines of extraneous code.

…eby axis values were being reassigned instead of the axis unit. Also added some error checks and removed a few lines of extraneous code.

…write test for it.

…urrent plot API. Also make those ImageAnimator classes aware of the sub-cubes data unit.

…r. Add back in after sunpy 0.9 release.

DanRyanIrish · 2018-03-26T19:51:28Z

I've left 1D animation functionality out of this PR as it depends on sunpy master. I'll issue another PR once this one has been merged with the 1D animation capability that can be merged either when sunpy 0.9 comes out or if we decide ndcube master depending on sunpy master is fine until 0.9.

DanRyanIrish · 2018-03-26T19:54:25Z

As part of this PR I've realised there are shortcomings in the 2D images produced, it that coordinates for an axis are sometimes taken from the first sub-cube in the sequence and assumed not to change. I haven't fully figured out all the ties this occurs. So this comment is just a reminder to raise an issue after this PR is merged. Addressing this issue is beyond the scope of this PR.

…e_plot_api

…plot_as_cube docstrings and add type checks in their tests.

…e_plot_api

DanRyanIrish · 2018-03-29T00:51:49Z

Hey @Cadair. I'm planning to merge this tomorrow (your today) as I think it's ready and there are a couple PRs piling up waiting for it. I know you're really busy so if you don't have time to look at it no problem. Otherwise I'll take any comments you do have into tomorrow.

Updated CHANGELOG to include PRs #98, #101 and #103.

First draft NDCubeSequence plottling mixin API.

96cb144

DanRyanIrish added 4 commits March 22, 2018 00:28

First working version of NDCubeSequence plotting mixin.

b9d1af8

Add NDCubeSequence ImageAnimator classes to plotting mixing.

4a53405

Fill in plot() function of NDCubeSequencePlotMixin and some minor kwa…

6835bcd

…rg API changes.

Create paths for line animation options in NDCubeSequencePlotMixin wi…

6b5a8c5

…th placeholder errors.

DanRyanIrish force-pushed the sequence_plot_api branch from ffda5b2 to 6b5a8c5 Compare March 22, 2018 20:06

DanRyanIrish changed the title ~~NDCubeSequence plottling mixin API.~~ NDCubeSequence plotting mixin API. Mar 22, 2018

DanRyanIrish added 2 commits March 22, 2018 17:33

Wrote LineAnimator classes for NDCubeSequence in sequence and cube-li…

d6b2c24

…ke dimension states. Also got NDCubeSequencePlotMixin.plot working with LineAnimatorNDCubeSequence class for 2D sequences.

Implement NDCubeSequence LineAnimator in NDCubeSequencePlotMixin for …

cdb6a1d

…cases where sequence dimensions > 2.

DanRyanIrish commented Mar 23, 2018

View reviewed changes

Cadair reviewed Mar 23, 2018

View reviewed changes

DanRyanIrish commented Mar 23, 2018

View reviewed changes

DanRyanIrish mentioned this pull request Mar 23, 2018

Enable 1D Animation of NDCube #81

Merged

DanRyanIrish added 3 commits March 23, 2018 10:59

Create place-holder for new top level NDCubeSequence plot_as_cube met…

31a6afa

…hod. Also updaed docstrings of plot and plot_as_cube to reflect the what the API will become over future commits.

Outlined close to (but not) working implementation for NDCubeSeqeunce…

6c698db

…PlotMixin.plot_as_cube.

Simpify implementation of NDCubeSequencePlotMixin.plot.

5910439

Move checking and prepping of some kwargs in NDCubeSequencePlotMixin …

cae5a0e

…plot and and plot_as_cube methods to external function. Also added more comments.

DanRyanIrish force-pushed the sequence_plot_api branch from 5ea7c04 to 94b9b2c Compare March 24, 2018 04:08

Remove unused functions.

4fa0da3

DanRyanIrish force-pushed the sequence_plot_api branch from 94b9b2c to 4fa0da3 Compare March 24, 2018 04:10

DanRyanIrish added 4 commits March 24, 2018 11:10

Enabled user to set data unit in LineAnimatorNDCubeSequence.

5fc66e5

Enable user to set data unit in LineAnimatorCubeLikeNDCubeSequence.

ccccaa9

Rewrote tiling of x-axis coords in LineAnimatorNDCubeSequence that wi…

f359ab5

…ll stop bugs when dimensions are of same length. Also made NDCubeSequencePlotMixin a proper class by replacing the cubesequence input arg to self.

Tidy up implementation of _prep_axes_kwargs() and remove eroneous sel…

7ece239

…f occurences.

DanRyanIrish added 9 commits March 25, 2018 22:09

Write tests for NDCubeSequencePlotMixin.plot and plot_as_cube 1D line…

f6e08f7

… plot options. Also implement mixin in ndcubesequence module creating a non-plotting Base class and an NDCubeSequence class using plot mixin.

Fix bus in NDCubeSequencePlotMixin._plot_sequence_2D whereby axis val…

8bf5036

…ues were being reassigned instead of the axis unit. Also added some error checks and removed a few lines of extraneous code.

Add tests for NDCubeSequencePlotMixin._plot_sequence_2D.

d02118a

Fix bug in NDCubeSequencePlotMixin._plot_3D_sequence_as_2D_image wher…

1226447

…eby axis values were being reassigned instead of the axis unit. Also added some error checks and removed a few lines of extraneous code.

Add tests for NDCubeSequencePlotMixin._plot_3D_sequence_2D_image.

643408c

Fix calling of animation in NDCubeSequencePlotMixin.plot_as_cube and …

3b7b86a

…write test for it.

Rework NDCubeSeqeunce ImageAnimator classes API to be more like the c…

3e4997d

…urrent plot API. Also make those ImageAnimator classes aware of the sub-cubes data unit.

Delete visualisation module as no longer used.

3d62c7f

Remove LineAnimator capability as it currently depends on sunpy maste…

99aef15

…r. Add back in after sunpy 0.9 release.

DanRyanIrish force-pushed the sequence_plot_api branch from 65fdf62 to cc59ce4 Compare March 26, 2018 18:42

PEP8 changes.

bfe945c

DanRyanIrish force-pushed the sequence_plot_api branch from cc59ce4 to bfe945c Compare March 26, 2018 18:43

DanRyanIrish added 4 commits March 28, 2018 11:09

Add matplotlib backend to travis for testing plots.

4eb102c

Merge branch 'master' of https://github.com/sunpy/ndcube into sequenc…

a21570e

…e_plot_api

Merge branch 'master' of https://github.com/sunpy/ndcube into sequenc…

d929fc0

…e_plot_api

Add description fo return object in NDCubeSequencePlotMixin.plot and …

f25e39d

…plot_as_cube docstrings and add type checks in their tests.

DanRyanIrish mentioned this pull request Mar 28, 2018

Tests for NDCube.plot and API Update #103

Merged

Merge branch 'master' of https://github.com/sunpy/ndcube into sequenc…

47f8ec3

…e_plot_api

DanRyanIrish force-pushed the sequence_plot_api branch 3 times, most recently from b80dc28 to 0917b02 Compare March 28, 2018 22:09

Set matplotlib backend for testing.

d629ec8

DanRyanIrish force-pushed the sequence_plot_api branch from 0917b02 to d629ec8 Compare March 28, 2018 22:17

DanRyanIrish merged commit 9ec7d20 into sunpy:master Mar 29, 2018

DanRyanIrish mentioned this pull request Mar 29, 2018

Make NDCubeSequence plotting a mixin #92

Closed

DanRyanIrish added a commit to DanRyanIrish/ndcube that referenced this pull request Mar 30, 2018

Updated CHANGELOG to include PRs sunpy#98, sunpy#101 and sunpy#103.

3568e71

DanRyanIrish added a commit that referenced this pull request Mar 30, 2018

Merge pull request #108 from DanRyanIrish/changelog_update

c19de57

Updated CHANGELOG to include PRs #98, #101 and #103.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NDCubeSequence plotting mixin API. #98

NDCubeSequence plotting mixin API. #98

DanRyanIrish commented Mar 21, 2018

pep8speaks commented Mar 21, 2018 •

edited

Loading

DanRyanIrish commented Mar 22, 2018

DanRyanIrish commented Mar 22, 2018

DanRyanIrish Mar 23, 2018

DanRyanIrish Mar 23, 2018

Cadair Mar 23, 2018

DanRyanIrish Mar 23, 2018

DanRyanIrish commented Mar 23, 2018

DanRyanIrish commented Mar 23, 2018

DanRyanIrish commented Mar 23, 2018

DanRyanIrish commented Mar 26, 2018

DanRyanIrish commented Mar 26, 2018

DanRyanIrish commented Mar 29, 2018

NDCubeSequence plotting mixin API. #98

NDCubeSequence plotting mixin API. #98

Conversation

DanRyanIrish commented Mar 21, 2018

pep8speaks commented Mar 21, 2018 • edited Loading

Comment last updated on March 26, 2018 at 18:43 Hours UTC

DanRyanIrish commented Mar 22, 2018

DanRyanIrish commented Mar 22, 2018

DanRyanIrish Mar 23, 2018

Choose a reason for hiding this comment

DanRyanIrish Mar 23, 2018

Choose a reason for hiding this comment

Cadair Mar 23, 2018

Choose a reason for hiding this comment

DanRyanIrish Mar 23, 2018

Choose a reason for hiding this comment

DanRyanIrish commented Mar 23, 2018

DanRyanIrish commented Mar 23, 2018

DanRyanIrish commented Mar 23, 2018

DanRyanIrish commented Mar 26, 2018

DanRyanIrish commented Mar 26, 2018

DanRyanIrish commented Mar 29, 2018

pep8speaks commented Mar 21, 2018 •

edited

Loading