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

Split pro forma module into multiple classes #26

Merged
merged 10 commits into from
Mar 3, 2017
Merged

Conversation

pksohn
Copy link
Contributor

@pksohn pksohn commented Mar 3, 2017

This PR partly addresses #17 and is purely an improvement to readability and maintainability, basically by refactoring the module into smaller chunks of code.

The main change is to create two new classes:

  • The SqFtProFormaReference class now handles all of the _generate_lookup code, which creates the reference tables (which create some metrics for each floor-to-area ratio, for each form and parking configuration) the the lookup method eventually uses. The main class generates this object and saves the tables upon instantiation (here).
  • The SqFtProFormaLookup class now handles all of the lookup code which actually returns the feasibility results. The main class just creates an object of this class and calls its lookup method.

Within each of these classes, I’ve moved some code out of long methods into their own helper methods wherever it seemed to make sense. For example:

  • In the SqFtProFormaReference class, the _building_bulk, _park_sqft, and _stories helper methods were all part of the original _generate_lookup method. Note that I also started to try and refer to this as the “reference” table to disambiguate from the “lookup” method later (both were referred to as “lookup” earlier).
  • Similarly, helper methods like _simple_zoning were split out of the lookup method.

A couple more minor things:

  • I thought the np.reshape(data, (-1,1)) code that was in several places was a little confusing so I made a small utility function called columnize that hopefully makes the code more readable.
  • Lots of minor changes to line lengths and such to comply with PEP-8

@pksohn pksohn requested a review from conorhenley March 3, 2017 19:33
@conorhenley conorhenley merged commit fdfabe4 into master Mar 3, 2017
@conorhenley conorhenley deleted the proforma branch March 3, 2017 21:36
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.

None yet

2 participants