-
Notifications
You must be signed in to change notification settings - Fork 398
Add drawcounties method #65
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
Conversation
What's the source of the counties shapefile? We should add a LICENSE file somewhere. Are there different resolutions available? |
I should have also mentioned that in this pull request I also added a linestyle argument to all the draw* methods. I wasn't sure why it wasn't there before, but thought it might be a nice addition for people to used dashed boundaries if they desired. (I figured having the ability to do dotted counties might be nice, and if I added it there, for consistency it should be elsewhere, too.) There are different resolutions available, but as far as I can tell, the resolution that the current files use is higher than even the "full" resolution boundary data. As for the license, I've had to do some digging. The files were originally placed on a GIS site and claim to have come from ESRI. Digging more on the ESRI website, I can't find the specific files, nor a license/distribution/redistribution statement. I will say that I've found these same files on several University FTP servers, but again...no luck on finding license information. There are always the US Census county shapfiles, but that's ~120MB, versus the 1MB of the files uploaded. I also noticed that I branched from my latlon branch instead of master and the latlon commits made it here. I'm not sure how you want me to remove that. I can close this pull request and submit a new one under a new branch, or I can just make a new commit removing the latlon keyword hack. |
Yes, please do remove the latlon part of the commit. Regarding the data source, we really do need to be careful about that. We need to say what the source is, and what license it can be redistributed under. It would also be nice to have multiple resolution versions, similar to the coastline data, but that's not crucial. |
This uses a new county shapefile dataset that is from the NOAA Coastal Geospatial Data Project (http://coastalgeospatial.noaa.gov/data_gis.html) I modified the .dbf file to clean it up a bit and shrink the size. |
Could you add the data source URL to the docstring? |
It looks like you only added the linestyle keyword to drawcounties, not the other draw* methods. Did you want to do that? |
That's pretty bad... I accept the keyword, but then don't do anything with it. The idea was to just add the drawcounties method in a manner like all the other methods with this pull request and then if you wanted to extend the linestyle option to all the other methods do so with a new pull request. However, if you want to go ahead and add the linestyle argument, I'll put those in this PR, clean it up, and repush. In any event, I'll add the URL information to the docstrings. |
OK - I guess I thought you said you had already added it, but maybe I misunderstood. I think it would be good to keep the interface consistent across all the draw* methods. |
Sorry, that was an error on my part. It had been included, but I removed it when I cleaned up the commits updating the shapefile datasets. I went ahead and created a new update, this time off the latest iteration of master. In it I:
At this point I think it should be good to go. |
Here is the first go at adding functionality for drawing counties on a map. It reads in a UScounties shapefile dataset, which has been added to the data directory.
One note is that the resolution of the default draw methods (coastlines, countries, states, etc) can be changed. Using the shapefile provides only one resolution. Thus when drawing counties on a low or intermediate map resolution, differences between counties/states, counties/countries, counties/coastlines can be found when zoomed in.