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

Adding optional counties to draw_map() #152

Closed
blychs opened this issue Sep 4, 2024 · 2 comments · Fixed by #153
Closed

Adding optional counties to draw_map() #152

blychs opened this issue Sep 4, 2024 · 2 comments · Fixed by #153
Assignees

Comments

@blychs
Copy link
Contributor

blychs commented Sep 4, 2024

Is your feature request related to a problem? Please describe.
There is no specific problem, this is an enhancement. I wanted to add the capability to plot counties.
I have already this coded in my fork and if you agree, I'd be happy to submit a PR

Describe the solution you'd like
The easiest option would be to have the optional parameter in plots.mapgen.draw_map, which would now be

def draw_map(
    crs=None,
    natural_earth=False,
    coastlines=True,
    counties=False,
    states=False,
    countries=True,
    resolution="10m",
    extent=None,
    figsize=(10, 5),
    linewidth=0.25,
    return_fig=False,
    **kwargs
):

Describe alternatives you've considered
I suggest simply adding something similar to states to draw_map, albeit

    if counties:
        counties = cfeature.NaturalEarthFeature(
            category='cultural',
            name='admin_2_counties',
            scale=resolution,
            facecolor='none',                                                                                                                                                                                                                                                    
            edgecolor='k',
            linewidth=linewidth,
            )  

Additional context
This is needed to make useful plots when it comes to evaluations done by specific states, since it is hard to recognize the region in a map that only includes a specific state without counties. The solution seems simple enough and I can't think of any drawbacks. I have already tested this addition in my fork and it seems to be working just fine.

@zmoon
Copy link
Member

zmoon commented Sep 5, 2024

@blychs sounds good, I agree it should be false by default.

@blychs
Copy link
Contributor Author

blychs commented Sep 5, 2024

Done, I'll close this as soon as the code gets reviewed and (hopefully) merged.

@blychs blychs closed this as completed Sep 5, 2024
@zmoon zmoon linked a pull request Sep 5, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants