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

Flowmachine event count #1023

Merged
merged 11 commits into from
Jul 3, 2019
Merged

Flowmachine event count #1023

merged 11 commits into from
Jul 3, 2019

Conversation

BhavinPanch
Copy link
Contributor

@BhavinPanch BhavinPanch commented Jul 2, 2019

Closes #992

I have:

  • Formatted any Python files with black
  • Brought the branch up to date with master
  • Added any relevant Github labels
  • Added tests for any new additions
  • Added or updated any relevant documentation
  • Added an Architectural Decision Record (ADR), if appropriate
  • Added an MPLv2 License Header if appropriate
  • Updated the Changelog

Event count values are exposed through FlowAPI's Joined Spatial Aggregate.

@greenape greenape added enhancement New feature or request FlowClient Issues related to FlowClient FlowMachine Issues related to FlowMachine labels Jul 2, 2019
Copy link
Member

@greenape greenape left a comment

Choose a reason for hiding this comment

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

Cool.

Can we add this to docs/source/developer/developer.md in the API Routes section around L77 as well please.

CHANGELOG.md Outdated Show resolved Hide resolved
start: str,
stop: str,
direction: str = "both",
event_types: Union[None, List[str]] = None,
Copy link
Member

Choose a reason for hiding this comment

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

Can actually write this as Optional[List[str]] which is equivalent to Union[None, ..] and maybe a bit clearer.

Co-Authored-By: Jonathan Gray <[email protected]>
@codecov
Copy link

codecov bot commented Jul 2, 2019

Codecov Report

Merging #1023 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1023      +/-   ##
==========================================
+ Coverage   93.93%   93.96%   +0.02%     
==========================================
  Files         141      142       +1     
  Lines        6930     6957      +27     
  Branches      693      693              
==========================================
+ Hits         6510     6537      +27     
  Misses        309      309              
  Partials      111      111
Flag Coverage Δ
#flowapi_unit_tests 82.4% <ø> (ø) ⬆️
#flowauth_unit_tests 94.86% <ø> (ø) ⬆️
#flowclient_unit_tests 81.69% <50%> (-0.31%) ⬇️
#flowetl_unit_tests 100% <ø> (ø) ⬆️
#flowkit_jwt_generator_unit_tests 100% <ø> (ø) ⬆️
#flowmachine_unit_tests 91.37% <72%> (-0.1%) ⬇️
#integration_tests 62.25% <100%> (+0.22%) ⬆️
Impacted Files Coverage Δ
...e/server/query_schemas/joined_spatial_aggregate.py 100% <100%> (ø) ⬆️
...owmachine/core/server/query_schemas/event_count.py 100% <100%> (ø)
flowclient/flowclient/client.py 94.36% <100%> (+0.05%) ⬆️

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 05ba372...4129b1c. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jul 2, 2019

Codecov Report

Merging #1023 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1023      +/-   ##
==========================================
+ Coverage   93.93%   93.96%   +0.02%     
==========================================
  Files         141      142       +1     
  Lines        6930     6957      +27     
  Branches      693      693              
==========================================
+ Hits         6510     6537      +27     
  Misses        309      309              
  Partials      111      111
Flag Coverage Δ
#flowapi_unit_tests 82.4% <ø> (ø) ⬆️
#flowauth_unit_tests 94.86% <ø> (ø) ⬆️
#flowclient_unit_tests 81.69% <50%> (-0.31%) ⬇️
#flowetl_unit_tests 100% <ø> (ø) ⬆️
#flowkit_jwt_generator_unit_tests 100% <ø> (ø) ⬆️
#flowmachine_unit_tests 91.37% <72%> (-0.1%) ⬇️
#integration_tests 62.25% <100%> (+0.22%) ⬆️
Impacted Files Coverage Δ
...e/server/query_schemas/joined_spatial_aggregate.py 100% <100%> (ø) ⬆️
...owmachine/core/server/query_schemas/event_count.py 100% <100%> (ø)
flowclient/flowclient/client.py 94.36% <100%> (+0.05%) ⬆️

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 05ba372...4129b1c. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 2, 2019

Codecov Report

Merging #1023 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1023      +/-   ##
==========================================
+ Coverage   93.93%   93.96%   +0.02%     
==========================================
  Files         141      142       +1     
  Lines        6930     6957      +27     
  Branches      693      693              
==========================================
+ Hits         6510     6537      +27     
  Misses        309      309              
  Partials      111      111
Flag Coverage Δ
#flowapi_unit_tests 82.4% <ø> (ø) ⬆️
#flowauth_unit_tests 94.86% <ø> (ø) ⬆️
#flowclient_unit_tests 81.69% <66.66%> (-0.31%) ⬇️
#flowetl_unit_tests 100% <ø> (ø) ⬆️
#flowkit_jwt_generator_unit_tests 100% <ø> (ø) ⬆️
#flowmachine_unit_tests 91.37% <72%> (-0.1%) ⬇️
#integration_tests 62.25% <100%> (+0.22%) ⬆️
Impacted Files Coverage Δ
...e/server/query_schemas/joined_spatial_aggregate.py 100% <100%> (ø) ⬆️
...owmachine/core/server/query_schemas/event_count.py 100% <100%> (ø)
flowclient/flowclient/client.py 94.36% <100%> (+0.05%) ⬆️

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 05ba372...ad2453a. Read the comment docs.

Copy link
Member

@greenape greenape left a comment

Choose a reason for hiding this comment

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

Couple of minor points, otherwise good.

CHANGELOG.md Outdated
@@ -28,6 +28,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Added a `Geography` query class, to get geography data for a spatial unit.
- FlowAPI's 'joined_spatial_aggregate' endpoint now exposes unique location counts.[#949](https://github.com/Flowminder/FlowKit/issues/949)
- FlowAPI's 'joined_spatial_aggregate' endpoint now exposes subscriber degree.[#969](https://github.com/Flowminder/FlowKit/issues/969)
- FlowAPI's 'joined_spatial_aggregate' endpoint now exposes event counts.[#992](https://github.com/Flowminder/FlowKit/issues/992)
Copy link
Member

Choose a reason for hiding this comment

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

Move up to unreleased pls.

@@ -136,6 +136,9 @@ At present, the following query types are accessible through FlowAPI:

Count of mobile phone cells per area active based on CDR traffic within a time period, broken down into time buckets.

-`event_count`

Count of event for individual subscribers in a time period.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Count of event for individual subscribers in a time period.
Count of events (optionally of specific types) for individual subscribers in a time period.

direction : {"in", "out", "both"}, default "both"
Optionally, include only ingoing or outbound calls/texts. Can be one of "in", "out" or "both".
event_types : list of str, optional
The event types for which to return available dates (for example: ["calls", "sms"]).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The event types for which to return available dates (for example: ["calls", "sms"]).
The event types to include in the count (for example: ["calls", "sms"]).

Optionally, include only ingoing or outbound calls/texts. Can be one of "in", "out" or "both".
event_types : list of str, optional
The event types for which to return available dates (for example: ["calls", "sms"]).
If None, return available dates for all available event types.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If None, return available dates for all available event types.
If None, include all event types in the count.

@greenape greenape added the ready-to-merge Label indicating a PR is OK to automerge label Jul 3, 2019
@mergify mergify bot merged commit 57a71dd into master Jul 3, 2019
@mergify mergify bot deleted the flowmachine-event-count branch July 3, 2019 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request FlowClient Issues related to FlowClient FlowMachine Issues related to FlowMachine ready-to-merge Label indicating a PR is OK to automerge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose EventCount via api
2 participants