-
Notifications
You must be signed in to change notification settings - Fork 53
Support the 'electiondate' argument when pulling trend reports #334
base: master
Are you sure you want to change the base?
Conversation
…passed API keys and specific election dates) and to better handle draws for test trend data.
… -- an election date argument. Factor out logic of attaching date to election via CLI, so it can be written once and then used in both date-required and date-allowed decorators.
…o they now consider API key, election date (via the newly-added decorator) and test-result arguments. Change how the trend-file argument gets passed to these.
…h positional args (named args already worked).
…7, Python 3.6 and PyPy.
…ort CLI commands.
… if the '-d/--data-file' argument is also set. This is identical to how the required-date decorator works.
@allanjamesvestal, thank you for this thoughtful patch. Would it be possible to add a test that verifies a past date is working? |
Can do! I'll add that in this afternoon. As I mentioned in the tome above, I have a second branch that fixes a couple pre-existing flake8 issues. It'd be a little time-consuming to extract those from the changes here, so I'd like to do that second if this is going to be able to be merged somewhat quickly. I did want to mention it here, though, because you may want to group the two together in the same version bump. |
Hey @palewire, so I'm writing that test now. It's a little more involved than the other tests around trend reports (it's a two-step process that involves getting the list of all reports, and then getting the actual report we want), so I wanted to run this by you. The good news is that I can pretty easily just The bad news is, that all requires Mock to be installed. Well, it's there for Python 3, but I'll need to add a line to Is that OK? |
…ibrary under 'unittest' in Python 3).
…rect election. Rename the test class contained in 'tests.test_trend_reports'.
Updated with two tests — one to ensure calling the loader without an They and all other tests pass for me under Python 2 & 3 and PyPy. |
Thanks for the additional patches. I'll try to give them a close look in the next few days. |
This branch adds a couple configuration options to
elex.api.trends.BaseTrendReport
, and also to the corresponding CLI tasks (elex.cli.app.ElexBaseController.{governor|house|senate}_trends
).Chief among these additions is support for an
electiondate
option in theBaseTrendReport
constructor, which filters the returned list of reports by election-date value. That enables users to get balance-of-power reports for several past elections. (The other added option isapi_key
for explicit key settings -- since the AP has required organizations including ours to use separate keys for national and state-by-state data in the past.)All arguments passed to the
BaseTrendReport
constructor now come in as*args, **kwargs
, which adds future flexibility. Consequently, the parameters that were already supported (trend_file
andtestresults
) retain their names and default values, but the logic by which they are set changes slightly.The
{governor|house|senate}-trends
CLI commands have been modified to accept — but not require — a date value. In prior releases, a date value wouldn't raise an error, but would be ignored. This date support has been added by creating a new CLI-command decorator that mimics the behavior ofelex.cli.decorators.require_date_argument
, but without erroring out if no date was set.(These three CLI commands continue to support the optional
trend_file
andtest
arguments as in previous versions. Arguments that are common to all the CLI tasks [batch_name
,debug
,format_json
,suppress_output
,output_handler_override
andwith_timestamp
] have all been tested and should still work properly as well.)These changes have been tested* using
make test
in Python 2.7.8 and 3.6.5, as well as in the latest Homebrew-installed PyPy. However, I haven't written any additional tests or documentation for these — nor have I bumped any version numbers insetup.py
or elsewhere (I wanted feedback on what's needed before I did so).Please let me know how these changes look. I'd love to get them incorporated before the election, if it's possible.
make test
actually fails due to a few linting issues that pre-date my forking of the repo. I've resolved them in a branch on my own forked repo, and will submit a second PR with these minor changes when/if this current PR gets merged