Skip to content

fix #397 needed to store continent value #398

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.

Sign up for GitHub

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

Merged
doutriaux1 merged 4 commits into from
Apr 1, 2019
Merged

fix #397 needed to store continent value #398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
20f721c
fix #397 needed to store continent value
doutriaux1 Mar 28, 2019
00bde52
trying to get correct pkg in for build to work (works on my mac)
doutriaux1 Mar 29, 2019
39948d8
typo
doutriaux1 Mar 29, 2019
d2f70be
sphinx-autoapi conflicts and i don't think we need it
doutriaux1 Mar 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ aliases:
if [[ $PY_VER = "py2" ]]; then
conda create -q -n $PY_VER $CUSTOM_CHANNELS $CHANNELS $PKGS $TEMP_PKGS "python<3"
else
conda create -q -n $PY_VER $CUSTOM_CHANNELS $CHANNELS $PKGS $TEMP_PKGS "python=3.6" nbsphinx easydev $COVERAGE_PKGS
conda create -q -n $PY_VER $CUSTOM_CHANNELS $CHANNELS $PKGS $TEMP_PKGS "python=3.6" sphinxcontrib-websupport nbsphinx easydev $COVERAGE_PKGS
fi

- &setup_vcs
Expand Down
9 changes: 9 additions & 0 deletions tests/test_vcs_update_triggers_continents.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import basevcstest

class TestVCSTextsExtents(basevcstest.VCSBaseTest):
def testUpdateDoesNotTriggerContinents(self):
s = self.clt["clt"][0]
self.x.plot(s, continents=0)
Copy link
Copy Markdown
Collaborator

@scottwittenburg scottwittenburg Mar 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I guess while I knew you could disable the continents somehow, I never knew this is how you did it.

self.x.setcolormap("viridis")
self.checkImage("test_vcs_update_triggers_continents.png")

2 changes: 2 additions & 0 deletions vcs/Canvas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4054,6 +4054,8 @@ def __plot(self, arglist, keyargs):
dn.g_name = arglist[4]
dn.array = arglist[:2]
dn.backend = returned_kargs
if "continents" in keyargs:
dn._continents = keyargs["continents"]
Copy link
Copy Markdown
Collaborator

@scottwittenburg scottwittenburg Mar 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After browsing the source code, I'm not seeing how this value stored on the display is ever queried to avoid plotting the continents. To me it looks like the decision is made inside our vcs2vtk.genGrid(...) function, based solely on the type of data we get. But I guess it's working, so it's fine with me.

Copy link
Copy Markdown
Contributor Author

@doutriaux1 doutriaux1 Mar 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right, it's sort of obscure but i think it's there: https://github.com/CDAT/vcs/blob/master/vcs/VTKPlots.py#L491

Copy link
Copy Markdown
Collaborator

@scottwittenburg scottwittenburg Mar 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you referred to it with an underscore in your fix, so I was searching for it that way, forgetting about how you typically make "properties" out of those attributes. Thanks.

else:
dn = None

Expand Down