search

CDAT / vcs-js

3 stars 3 forks source link

Workaround for isoline plot error from web #37

Closed scottwittenburg closed 6 years ago

scottwittenburg commented 6 years ago

@danlipsa @doutriaux1 @James-Crean This fixes the issue plotting isolines in vcdat, at least for me.

I'm still not sure why we have two functions called setgraphicsmethod in there, but at least now they both invoke the same underlying code. Down the road, I can figure how to get rid of one of those. Also, it would be nice to get rid of that code which checks for specific values like 100000000000000000000, but it has been sitting there this long, and so can probably sit a little longer until we fix the default values issue (setting values to 1e20) in vcs. At least now it works and isn't duplicated.

I was worried this change might have broken the taylor diagram in vcdat, but going back to master that seems broken anyway, there's just a bit more information in the log now.

scottwittenburg commented 6 years ago

Since @doutriaux1 is looking at how to remove numeric defaults in general, I felt we should confine the current workaround to vcs-js. Then once we have a better system for identifying unspecified values, we get rid of all the special value checks here at one time.

danlipsa commented 6 years ago

@scottwittenburg @doutriaux1 I am fine with not changing vcs in this PR. The question is should we add this additional special value in vcs.js? I see this value is only used for initialization - not sure why? Maybe simply as a max_float?

scottwittenburg commented 6 years ago

I'm not sure I fully understand your question. If we don't add this in vcs-js, then the isoline plotting still doesn't work. We needed to support two things in the vcs-js special value checking:

  1. Deeper nesting of list element checking. Since the levels field in this case was a 2D array, we didn't get inside that inner array and check for it's special values.

  2. Checking not just for 100000000000000000000, but also the special value used by the isoline plot.

Only with those two additions does this "workaround" and actually work. But maybe I'm just misunderstanding your concerns?

danlipsa commented 6 years ago

I see. Looks good then.