Skip to content

Conversation

nparley
Copy link
Contributor

@nparley nparley commented Jul 21, 2016

When calling drawlsmask for a second time _readlsmask will not reload the data. However, cylproj was still be calculated and because lsmask_lons could be None this will cause an error. This code does not need to be run if self.lsmask is not None so it's most likely a tab error and it should belong to the if self.lsmask is None: block.

@WeatherGod
Copy link
Member

Hmmm, this logic is still wrong, though. If I am reading this right, if someone passes in a new lsmask after already calling this function, then it doesn't get used, and it doesn't get adjusted for cyclic projections. Makes me wonder if no one passes in their own lsmask?

I would imagine that the correct way to deal with this is to set self.lsmask to None at the start of the function if a lsmask is passed in (and that lsmask is determined to be different from the current one). Then the rest of the logic is simplified.

@nparley
Copy link
Contributor Author

nparley commented Jul 21, 2016

Maybe adding this in?:

# Clear saved lsmask is new lsmask is passed
if lsmask is not None or lsmask_lons is not None or lsmask_lats is not None:
  self.lsmask = None

@WeatherGod
Copy link
Member

yeah, that should do the trick along with your current set of changes

@@ -3901,6 +3901,9 @@ def drawlsmask(self,land_color="0.8",ocean_color="w",lsmask=None,
# look for axes instance (as keyword, an instance variable
# or from plt.gca().
ax = kwargs.pop('ax', None) or self._check_ax()
# Clear saved lsmask is new lsmask is passed
if lsmask is not None or lsmask_lons is not None or lsmask_lats is not None:
self.lsmask = None
Copy link
Member

Choose a reason for hiding this comment

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

I might do one final little check. I could see adding a check to see if the passed in lsmask is the same as the cached lsmask.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point

if lsmask is not None or lsmask_lons is not None \
or lsmask_lats is not None:
# Make sure passed lsmask is not the same as cached mask
if lsmask != self.lsmask:
Copy link
Member

Choose a reason for hiding this comment

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

Don't use equality. Use is not instead when comparing objects.
Also, slight typo in the comments above "lsmask is new" --> "lsmask if new"

@WeatherGod WeatherGod merged commit 26cfbef into matplotlib:master Aug 8, 2016
@WeatherGod
Copy link
Member

Thanks for your fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants