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

Streamline API call flow (#142) #143

Closed
wants to merge 22 commits into from
Closed

Streamline API call flow (#142) #143

wants to merge 22 commits into from

Conversation

ronnie-llamado
Copy link
Member

Closes #142

cenpy/product.py Outdated

@lazy_property
def _legislative_year(self):
return 'NotImpementedError'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be raise NotImplementedError() rather than return the string?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in this case. _legislative_year is technically called on every layer lookup, but only formats if {legislative_year} is present in the string. If it raises an Exception, every query will fail.

        layername_s = self._layer_lookup[for_geography].format(
            census_year=self._census_year,
            legislative_year=self._legislative_year,
            congressional_district=self._congressional_district,
        )

Not sure if this was the best method, but it was the quickest. An alternative would to search for the placeholder and replace if present. Then we can easily raise exceptions.

@ronnie-llamado
Copy link
Member Author

ronnie-llamado commented May 16, 2021

I think this pull request might be ready for an initial review and potentially a dev branch. This pull request contains the bulk of the logic for the queries and has "100"% test coverage (for now).

There are still a handful of items that need to be bolted on in the near future:

  • support backwards compatibility (if 1.1, maybe not if 2.0?)
  • integrate fuzzywuzzy (rapidfuzz) into to product query
  • better documentation and exception handling

Working Geographies

supported total % geographies reference
acs 38 87 43.7 geographies
decennial 34 163 23.3 geographies

Whole Geographies

Almost all whole geographies are currently working. There are also three specific whole geographies cases that didn't work:

  1. ACS, Geography Level #860, state› zip code tabulation area
  2. Decennial, Geography Level #352, new england city and town area› principal city
  3. Decennial, Geography Level #560, state› congressional district› alaska native regional corporation

These three are similar to or part geographies since they require identifying multiple geographies and creating two unique queries from two different map service layers.


or part Geographies

All or part geographies are currently unsupported. It's a bit tricky to come up with a clean modular solution. In particular, the Decennial geographies are difficult to cover all cases. This can be tackled in the future, if the desire is there.

@ronnie-llamado ronnie-llamado self-assigned this May 16, 2021
@ronnie-llamado ronnie-llamado marked this pull request as ready for review May 16, 2021 06:18
@ronnie-llamado ronnie-llamado closed this by deleting the head repository Dec 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Streamline API call flow
2 participants