-
Notifications
You must be signed in to change notification settings - Fork 70
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
v2 broke us.states.lookup('District of Columbia')
#50
Comments
Looks like 6d21643 indicates that this change was intentional... Perhaps |
+1 to @wwoods. Not having DC anywhere in the mix in v2 (either in state.TERRITORIES or state.STATE or state.OBSOLETE) is a breaking change for me. In social scientist and data scientist applications, state-level data has DC listed in the state column of public datasets. |
I'll add another +1 — I just can't fathom what kind of intentional design change would remove support for I guess I'll submit a question to @jcarbaugh, @mileswwatkins and the team: if D.C. isn't a "territory" in the nomenclature of this project, what is it? As is, I'd imagine this represents a breaking change for a lot of people's use cases on this otherwise fantastic library. I'd hope its authors can fix this quickly within the 2.0.x series. Edit: I do want to clarify that my complaint is separate from whether to include D.C. in the My quibble is with the fact that D.C. now appears to be the one area not searched by |
Removing DC from I was very hesitant to remove DC from STATES, but got multiple requests and I think even a PR to remove it, so I went ahead and did it. I’m already working on a branch for 3.0 that was going to restore some support for DC as a STATE, but am not inclined to include it back in 2.x since its presence/removal is such a breaking change. Questions for those of you that have commented here:
|
Thank you for the quick response on that, @jcarbaugh. Again, I definitely understand the decision to remove D.C. from For us, we're only running Python 3, so we could upgrade to a 3.0 release on just about all our stuff without much headache. I'm intrigued by the idea of an environment variable for D.C. statehood, but my suspicion is that if (I will say, though, I saw your response to Ben Welsh's related tweet, and as a former Kentuckian/current Virginian I'd love to see a Thanks again for replying so quickly. We're especially interested in this since we have a similar library for election-specific metadata (us-elections) that relies on your project for much of its basic mappings. |
I just pushed v2.0.2 to PyPI. It adds DC back to I'm still thinking through the right approach to bringing DC back to STATES in some way in v3.0. @allanjamesvestal I'm happy to report that v3 will have a COMMONWEALTHS list haha https://github.com/unitedstates/python-us/blob/master/us/states.py#L1424 |
@jcarbaugh The most unique value contribution I found in this package was the ability to roll a custom dict so I can set keys in either direction for abbreviations (e.g.: us.states.mapping('name', 'abbr')). That functionality appears to be restored so I'm a happy camper. Many thanks for the rapid fix. Yes this is python 3.0 and yes some sort of "include dc" variable that I can set at the top of a notebook seems fine for my applications. |
@jess-peck-zocdoc those mappings were the original reason I created this package! I'm really not familiar with best practices when working with notebooks. Is it possible and viable to require that something like |
I think I don't really want to set environment variables in a notebook if I can avoid it. I'm happy to set a python global though. I have plenty of packages that require an import followed by a declaration of some option or preference in a subsequent call to behave the way I want. In jupyter notebooks locally this might be fine, but running in arbitrary containers or clusters I might not get to change everything about the python env that I might wish (see: databricks). |
Re: @jcarbaugh and @jess-peck-zocdoc -- It's probably better to have something like Python 3 on my end. Thanks for the quick fix push, @jcarbaugh ! |
Thanks for the fix, @jcarbaugh! I think that'll be a real boon to a lot of us. I have some (probably hastily formed) thoughts about the question of what all is included in At that point, I think there are two reasonable options. Either you have a way to specify zero, one or multiple non-states as being part of If you did want to have a configuration option for D.C. (and others), I'd definitely plus-one the suggestion to make it a method rather than an environment variable, the way @wwoods suggested. Any rate, I greatly do appreciate your quick response in shipping |
Glad to hear that this fix meets your immediate needs! Appreciate your help here. @allanjamesvestal @wwoods @jess-peck-zocdoc would you mind if I reached out to you all directly outside of this thread to discuss DC + other behavior for v3? I'd like to better understand your use cases and how the package is being used (e.g. Jupyter notebooks). I think I need to go either in an "assemble it yourself" way or via configuration, which I think would necessitate a significant API change. |
Happy to talk about this off-thread if I can be useful! |
Noting this as an issue because
us.states.DC
still exists, so there's no reasonus.states.lookup
should have broken.The text was updated successfully, but these errors were encountered: