Skip to content
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 filter for boolean values in dict2string so "no_rot=True" works (issue #183) #184

Merged
merged 6 commits into from
Mar 11, 2019

Conversation

jswhit
Copy link
Collaborator

@jswhit jswhit commented Mar 11, 2019

No description provided.

Copy link
Member

@snowman2 snowman2 left a comment

Choose a reason for hiding this comment

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

@jswhit, I think not including the option if false would be better.

@snowman2
Copy link
Member

Also, what about if they use None for the value?

@jswhit
Copy link
Collaborator Author

jswhit commented Mar 11, 2019

handles False and None now (None is treated like False, is that correct?)

@snowman2
Copy link
Member

snowman2 commented Mar 11, 2019

I was thinking None could be considered analagous to True as the None would be considered the same as not having a value for the argument. Thoughts on this logic?

...
if value is None or value is True:
    pjargs.append("+" + key + " ")
elif value is False:
    pass
...

@snowman2
Copy link
Member

Just updated the code ^^

@jswhit
Copy link
Collaborator Author

jswhit commented Mar 11, 2019

Or None might be interpreted as don't have a no_rot flag added to the string....

@snowman2
Copy link
Member

snowman2 commented Mar 11, 2019

It seems cartopy interprets None as not having a value.
See: https://github.com/SciTools/cartopy/blob/45d0479abed27f73c5bb8bac35036772a3faa773/lib/cartopy/_crs.pyx#L177

@jswhit
Copy link
Collaborator Author

jswhit commented Mar 11, 2019

OK, changed it to your suggestion

@snowman2
Copy link
Member

Sounds good, thanks for fixing this 👍

@jswhit jswhit merged commit 9b26e69 into master Mar 11, 2019
@jswhit jswhit deleted the issue183 branch March 11, 2019 18:38
@TAlonglong
Copy link

Thank you for this quick respond to my issue.

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.

3 participants