CDAT / vcs-js

3 stars 3 forks source link

Add functions to create/get/set graphics methods. #18

Closed danlipsa closed 6 years ago

danlipsa commented 6 years ago

@mattben @doutriaux1 @James-Crean Please review. This needs https://github.com/UV-CDAT/vcs/pull/270

doutriaux1 commented 6 years ago

@danlipsa I closed UV-CDAT/vcs#270 before I see this, the function you need already exsited I think, let me review this in detail.

danlipsa commented 6 years ago

@doutriaux1 I am using graphicsmethodlist now. Please review.

doutriaux1 commented 6 years ago

@danlipsa looks good, I'll run the demo.js before merging. One comment, why did you reverse template/method in plot function? It doesn't really matter, it's just that in vcs.plot() (Python) you're supposed to send x.plot(data1,data2,template_name, gm_type, gm_name) template first.

Anyhow should be merging this shortly.

doutriaux1 commented 6 years ago

@danlipsa while run npm run test

i get this:

13 11 2017 09:26:57.410:INFO [karma]: Karma v0.13.22 server started at http://localhost:9876/
13 11 2017 09:26:57.424:INFO [launcher]: Starting browser PhantomJS
13 11 2017 09:26:58.357:INFO [PhantomJS 2.1.1 (Mac OS X 0.0.0)]: Connected on socket M4yKYGR8uEJIcFkTAAAA with id 95943044
PhantomJS 2.1.1 (Mac OS X 0.0.0) ERROR
  Error: Cannot find module "inject!vcs/session"
  at /git/vcs-js/test/entry.js:5974 <- webpack:///test/cases/session.js:3:0

PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 0 of 0 ERROR (0.397 secs / 0 secs)

npm ERR! Darwin 16.7.0
npm ERR! argv "/Users/doutriaux1/anaconda2/envs/dev-nox/bin/node" "/Users/doutriaux1/anaconda2/envs/dev-nox/bin/npm" "run" "karma"
npm ERR! node v6.10.3
npm ERR! npm  v3.10.10
npm ERR! code ELIFECYCLE
npm ERR! vcs-js@0.1.0 karma: `karma start --single-run`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the vcs-js@0.1.0 karma script 'karma start --single-run'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the vcs-js package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     karma start --single-run
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs vcs-js
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls vcs-js
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     /git/vcs-js/npm-debug.log

npm ERR! Darwin 16.7.0
npm ERR! argv "/Users/doutriaux1/anaconda2/envs/dev-nox/bin/node" "/Users/doutriaux1/anaconda2/envs/dev-nox/bin/npm" "run" "test"
npm ERR! node v6.10.3
npm ERR! npm  v3.10.10
npm ERR! code ELIFECYCLE
npm ERR! vcs-js@0.1.0 test: `npm run lint && npm run karma`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the vcs-js@0.1.0 test script 'npm run lint && npm run karma'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the vcs-js package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     npm run lint && npm run karma
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs vcs-js
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls vcs-js
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     /git/vcs-js/npm-debug.log
doutriaux1 commented 6 years ago

and this before:

ERROR in ./test/cases/session.js
Module not found: Error: Cannot resolve 'file' or 'directory' /git/vcs-js/src/session in /git/vcs-js/test/cases
 @ ./test/cases/session.js 7:22-51

ERROR in ./src/file.js
Module not found: Error: Cannot resolve 'file' or 'directory' ./promise in /git/vcs-js/src
 @ ./src/file.js 11:153-173
webpack: Failed to compile.
danlipsa commented 6 years ago

@doutriaux1 Yes, the tests don't work. @jbeezley set them up when he created the stub for vcs.js and I have not run them since then. I plan to fix them with Jon's help. For now, I do testing in demo.js.

doutriaux1 commented 6 years ago

ah ok! Thx.

danlipsa commented 6 years ago

One comment, why did you reverse template/method in plot function?

To make it consistent with vcs.js where you send the gm and then the template I think.

It doesn't really matter, it's just that in vcs.plot() (Python) you're supposed to send x.plot(data1,data2,template_name, gm_type, gm_name) template first.

I didn't know that. In a way, it makes more sense to send the graphics method first as you are more likely to change that than the template. I am glad it does not matter. 😄