-
Notifications
You must be signed in to change notification settings - Fork 20
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
Fix adding textbox in the add_points method #44
Conversation
font_file = params.pop('font') | ||
font_size = int(params.pop('font_size', DEFAULT_FONTSIZE)) | ||
|
||
symbol = params.pop('symbol', DEFAULT_SYMBOL) | ||
pt_size = int(params.pop('pt_size', DEFAULT_PTSIZE)) | ||
ptsize = int(params.pop('ptsize', DEFAULT_PTSIZE)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this change? point_size
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It just make the keys from the config file consistence with the arguments in the function interface to avoid confusion. Similar as changing from 'list' to 'points_list' as we discussed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You changed all of them though. The kwarg here, the argument name in add_points
, and the key in the config. With this change it isn't consistent with the cities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the confusion is form that the keys ini config file for the add_cities() are not consistence with the add_cities() function. like 'list' vs 'citylist'; 'pt_size' vs 'ptsize'. i was trying to make they consistence in the add_points() case. If you think it is better to keep the 'pt_size' in the ini files, I can change them back.
Are there some news regarding this fix? Still waiting to use the new points feature with working text box backgound on all points. 👍 |
Thanks @yufeizhu600! |
Thanks for reviewing.@djhoese |
git diff origin/master **/*py | flake8 --diff
This PR fixed a bug that only apply the textbox parameters to the first point in the points_list in the add_points() function. Thanks to @peters77 for reporting the bug.
It also change the config keys 'list' and 'pt_size' in the config file for add_points() to make them consistence with the arguments of the function.
An example of using add_overlay_from_config() to apply overlays to a image is also added into the documents as suggested by @peters77.