dssg / collate

Aggregation SQL Query Builder
Other
1 stars 3 forks source link

Add validation and support for restricting the "beginning of time" #74

Closed mbauman closed 7 years ago

mbauman commented 7 years ago

Adds a new keyword parameter for SpacetimeAggregations that enables restricting the rows included in their calculations based upon an absolute minimum date.

Adds actual behavior tests for SpacetimeAggregation with testing.postgresql.

codecov-io commented 7 years ago

Codecov Report

Merging #74 into master will decrease coverage by <.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #74      +/-   ##
==========================================
- Coverage   95.53%   95.53%   -0.01%     
==========================================
  Files           4        4              
  Lines         269      291      +22     
==========================================
+ Hits          257      278      +21     
- Misses         12       13       +1
Impacted Files Coverage Δ
collate/spacetime.py 100% <100%> (ø) :arrow_up:
collate/collate.py 93.46% <100%> (-0.25%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 48def77...9b944f7. Read the comment docs.

mbauman commented 7 years ago

I've added a validation method for Aggregations.

This takes a SQL connection to allow validation against a SQL server. By default, Aggregation.execute() will call the validate method. SpacetimeAggregations now raise an error in the case where a date/interval combination happens to cross before the beginning of time (so long as the interval is not all).

If this proves to be annoying, we can perhaps change it to be a warning or even add an optional override to explicitly allow this to occur. But I think it behooves us to start conservatively.

mbauman commented 7 years ago

Alright, I've renamed the argument name to input_min_date. I don't think we need to really change other argument names at this time.