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

feat: update IC filter logic for duration_constraint. #66

Merged
merged 3 commits into from
Jul 22, 2024

Conversation

anujsinha3
Copy link
Collaborator

@anujsinha3 anujsinha3 commented Jul 22, 2024

#46 #67

  • Update the incremental clustering algorithm to use duration_constraint-based filtering for post-stay determination.
    An additional description has been highlighted in the linked issue.

  • Working Solution for the cases when the 'unc' column is missing/optional in the input.

@anujsinha3 anujsinha3 self-assigned this Jul 22, 2024
Copy link
Member

@carlosgjs carlosgjs left a comment

Choose a reason for hiding this comment

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

One inline suggestion.

"""
Returns the lat and long of the cluster to which the trace(row) belongs.
"""
if dur_constr:
lat_long = (row[STAY_LAT], row[STAY_LONG])
unc = row[STAY_UNC]
Copy link
Member

@carlosgjs carlosgjs Jul 22, 2024

Choose a reason for hiding this comment

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

Could this be simplified by adding STAY_UNC column with default value -1 when the file is initially loaded if the column is not present? I'm not sure if such columns are stored in a memory efficient way by pandas but probably worth trying. It would remove the need to pass around the df_columns and to have these special cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, was thinking the same. Will have to crosscheck for accuracy and timing as well.

Meanwhile, created an issue to explore this separately.

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.

2 participants