Skip to content

refactor basemaps #1075

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

Merged
merged 4 commits into from
May 25, 2022
Merged

refactor basemaps #1075

merged 4 commits into from
May 25, 2022

Conversation

12rambau
Copy link
Member

@12rambau 12rambau commented May 25, 2022

loops in dict

I decided to use values and items method from dictionaries to loop inside them.

  • First advantage is to reduce the length of the call and remove some useless lines
  • Second advantage is that it's clearer what key and item represent in the dict

get_xyz_dict

I used a more pythonic recursive function to retreive everything. Cherry on the cake it's way faster (1.1 ms on my machine).

PS

I did the modification on the fly in GitHub as I was passing by, I think it will require some testing before merging I may have done typos

12rambau added 3 commits May 25, 2022 17:00
I decided to use values and items method from dictionaries to loop inside them. 
First advantage is to reduce the length of the call and remove some useless lines
Second advantage is that it's clearer what `key` and `item` represent in the dict
@12rambau 12rambau changed the title use values and items to loop in dicts refactor basemaps May 25, 2022
using pop was dropping the max_zoom for all the layers
@lgtm-com
Copy link

lgtm-com bot commented May 25, 2022

This pull request introduces 1 alert when merging 077c8b2 into c219cd8 - view on LGTM.com

new alerts:

  • 1 for Modification of parameter with default

@giswqs
Copy link
Member

giswqs commented May 25, 2022

Looks great! Many thanks for improving the code quality. Much appreciated.

@giswqs giswqs merged commit c114e65 into gee-community:master May 25, 2022
@12rambau 12rambau deleted the patch-1 branch May 25, 2022 21:12
giswqs added a commit that referenced this pull request May 28, 2022
@giswqs
Copy link
Member

giswqs commented May 28, 2022

I just noticed that the refactored get_xyz_dict() function messed up the docs. I had to change the default value of both parameters to None.

https://geemap.org/basemaps/

def get_xyz_dict(free_only=True, _collection=xyz, _output={}):
def get_xyz_dict(free_only=True, _collection=None, _output=None)

Peek 2022-05-28 14-37

@12rambau
Copy link
Member Author

Oops sorry, glad to see that this modification was sufficient.

I'm using Shinx and autodoc for my libs and parameter/members/methods/functions prefixed by a "_" are ignored. I thought it was the same on your side.

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