New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test for BUG # 1728: wrapping data creates long cells #1830

Merged
merged 1 commit into from Feb 18, 2016

Conversation

Projects
None yet
3 participants
@danlipsa
Contributor

danlipsa commented Feb 15, 2016

No description provided.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 15, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 15, 2016

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 16, 2016

@danlipsa I only see an added test.

@danlipsa danlipsa force-pushed the wrapping-long-cells branch from 1ea219a to 1676bfc Feb 16, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 16, 2016

@doutriaux1 Indeed this is a test we wanted to add since we fixed this bug (adding +over to proj4).

@danlipsa danlipsa force-pushed the wrapping-long-cells branch from 1676bfc to fb0e4e4 Feb 16, 2016

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Feb 16, 2016

@danlipsa this is cool! so it works now? (I know I am asking obvious here :))

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 16, 2016

I see! Ok then can you please clean up the test. I will annotate what's wrong with it.

cdmsfile = cdms2.open(os.path.join(vcs.sample_data, "clt.nc"))
clt2 = cdmsfile('clt')
clt3 = clt2(latitude=(-90.0, 90.0),squeeze=1,longitude=(-180, 200.0),time=('1979-01', '1988-12'),)
axesOperations = eval("{'latitude': 'def', 'longitude': 'def', 'time': 'def'}")

This comment has been minimized.

@doutriaux1

doutriaux1 Feb 16, 2016

Member

useless

elif axesOperations[axis] == 'gtm':
clt3 = genutil.statistics.geometricmean(clt3, axis='(%s)'%axis)
elif axesOperations[axis] == 'std':
clt3 = genutil.statistics.std(clt3, axis='(%s)'%axis)

This comment has been minimized.

@doutriaux1

doutriaux1 Feb 16, 2016

Member

the whole loop is useless

gmBoxfill = vcs.getboxfill('a_robinson_boxfill')
args = []
args.append(clt3)
gmBoxfill.boxfill_type = 'linear'

This comment has been minimized.

@doutriaux1

doutriaux1 Feb 16, 2016

Member

all these are unmodified, you can remove these gmBoxfill lines bellow

clt3 = genutil.statistics.std(clt3, axis='(%s)'%axis)
canvas = vcs.init()
gmBoxfill = vcs.getboxfill('a_robinson_boxfill')
args = []

This comment has been minimized.

@doutriaux1

doutriaux1 Feb 16, 2016

Member

no need to use the args

kwargs[ 'cdmsfile' ] = cdmsfile.id
kwargs['bg'] = 1
args.append( gmBoxfill )
canvas.plot( *args, **kwargs)

This comment has been minimized.

@doutriaux1

doutriaux1 Feb 16, 2016

Member

canvas.plot(clt3,gmBoxfill) would do just as nice here

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 16, 2016

@aashish24 Yes, it did work since we added the +over option to proj4.

@danlipsa danlipsa force-pushed the wrapping-long-cells branch from fb0e4e4 to a4e51e4 Feb 16, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 16, 2016

@doutriaux1 I cleaned up the test as you suggested.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 16, 2016

@danlipsa L#17-#47 can still be removed, all of the gmBoxfill attribute setting lines

@danlipsa danlipsa force-pushed the wrapping-long-cells branch from a4e51e4 to 4873300 Feb 17, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 18, 2016

@doutriaux1 Any more comments? I think this is ready to be merged.

doutriaux1 added a commit that referenced this pull request Feb 18, 2016

Merge pull request #1830 from UV-CDAT/wrapping-long-cells
Add test for BUG # 1728: wrapping data creates long cells

@doutriaux1 doutriaux1 merged commit 6522555 into master Feb 18, 2016

4 of 6 checks passed

cont-int/LLNL/Linux-RH6 (FULL) 'ctest -j5 -D Experi' (Tue Feb 16 19:27:42 2016)
Details
continuous-integration/kitware-buildbot/uvcdat-garant-linux-release/ Build done.
Details
cont-int/LLNL/Linux-RH6 (MESA) 'ctest -j12 -D Exper' (Tue Feb 16 19:21:09 2016)
Details
cont-int/LLNL/Linux-Ub. 15.10 (FULL/MESA) 'git reset --hard ori' (Tue Feb 16 18:53:54 2016)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@doutriaux1 doutriaux1 deleted the wrapping-long-cells branch Feb 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment