Fix Callback Error Updating store-trips.data - TypeError: 'float' object is not subscriptable fixed

[TeachMeTW]

Issue: Callback Error Updating store-trips.data - TypeError: 'float' object is not subscriptable

Description

We are encountering a TypeError while updating store-trips.data. The error traceback indicates that there is an issue with the function applied to the ble_sensed_summary column in the DataFrame. The error occurs because a float object is being treated as if it were a dictionary.

Traceback

Callback error updating store-trips.data Traceback (most recent call last): File "/usr/src/app/app_sidebar_collapsible.py", line 318, in update_store_trips df, user_input_cols = query_confirmed_trips(start_date, end_date, timezone) File "/usr/src/app/utils/db_utils.py", line 101, in query_confirmed_trips df["data.primary_ble_sensed_mode"] = df.ble_sensed_summary.apply(get_max_mode_from_summary) File "/root/miniconda-23.5.2/envs/emission/lib/python3.9/site-packages/pandas/core/series.py", line 4771, in apply return SeriesApply(self, func, convert_dtype, args, kwargs).apply() File "/root/miniconda-23.5.2/envs/emission/lib/python3.9/site-packages/pandas/core/apply.py", line 1123, in apply return self.apply_standard() File "/root/miniconda-23.5.2/envs/emission/lib/python3.9/site-packages/pandas/core/apply.py", line 1174, in apply_standard mapped = lib.map_infer( File "pandas/_libs/lib.pyx", line 2924, in pandas._libs.lib.map_infer File "/usr/src/app/utils/db_utils.py", line 97, in <lambda> get_max_mode_from_summary = lambda md: max(md["distance"], key=md["distance"].get) if len(md["distance"]) > 0 else "INVALID" TypeError: 'float' object is not subscriptable

Issue

The TypeError occurs because the lambda function get_max_mode_from_summary expects md to be a dictionary with a "distance" key that maps to another dictionary. However, it appears that some entries in ble_sensed_summary are float values, which are not subscriptable and thus lead to this error.

Steps to Reproduce

1. Call the update_store_trips function with appropriate parameters.
2. Observe the error traceback indicating the TypeError.

Expected Behavior

The ble_sensed_summary column should be processed correctly, and the get_max_mode_from_summary function should handle various data types properly without raising an error.

Suggested Fix

Update the get_max_mode_from_summary lambda function to handle cases where md is not a dictionary. For example:

get_max_mode_from_summary = lambda md: (
    "INVALID" if not isinstance(md, dict) or "distance" not in md or not isinstance(md["distance"], dict) else
    (max(md["distance"], key=md["distance"].get) if len(md["distance"]) > 0 else "INVALID")
)

Consider adding data validation or cleaning steps before applying the lambda function.

[TeachMeTW]

Utilized Black Formatter (for formatting purposes)
Modified mongodump

[TeachMeTW]

Use Case of Updated Script
./load_mongodump.sh ~/Downloads/openpath_prod_ca_ebike_may_29.tar.gz Stage_database op-admin-dashboard-db-1 openpath_prod_ca_ebike

[JGreenlee]

The error handling is great.

As you have seen, this project doesn't use Black or adhere to PEP8. Although that's not ideal, could you revert the formatting changes for now?
Standardized code format is great but when formatting changes are done at the same time as other changes, it is harder to review

If we were to enforce a code style, we would need to decide on it as a team, then update the whole codebase at once, independently from other changes
(we did this on e-mission-phone using Prettier, the JS equivalent of Black)

When I make changes to python code, I do try to adhere to PEP8 on the lines I change. But I don't worry about the rest of the file

[shankari]

My comments are only about the load_mongodump part

[shankari]

@TeachMeTW can you pull out the loadmongodump changes into a separate PR (e.g. use git checkout to an earlier point and make a new branch)? I am happy to merge that now.

For the ble_summary, I would like to make sure that your approach is consistent with Jack and Abby's, so that can come in a follow-on PR.

[shankari]

Since I merged #122 to master, this only has the actual functionality changes. @JGreenlee and @Abby-Wheelis, I will let you review this before it comes to me.

[Abby-Wheelis]

This is the same functionality that I had to update in #141. Your checks are much more thorough than mine, so I'll go ahead and update to match this, but I set the result to UNKNOWN instead of INVALID, and I'm wondering if we should do the same thing in both places? I chose UNKNOWN since that is what trips where we were unable to sense a mode for most of the distance are displayed as, what purpose does INVALID serve here? Or maybe, what is the difference between an INVALID sensed mode and an UNKNOWN sensed mode?

[TeachMeTW]

@Abby-Wheelis I am not entirely sure what INVALID would represent here as opposed to unknown. I believe @shankari was the one who originally wrote this line of code that I refactored. I can change it to UNKNOWN to match #141

[shankari]

@TeachMeTW can you look at the commit where I wrote that code and see if I added in an explanation of why my choice? I have some vague recollection of the decision making, but the comments I wrote at the time are likely to be more clear.

[TeachMeTW]

@shankari @Abby-Wheelis based on existing comments, this is how I interpreted it.

INVALID: used to signify that the data or result is not just unknown but invalid. It implies that something went wrong or the data provided does not meet the expected format or criteria. It's a more explicit signal that the data is unusable.

UNKNOWN: indicates that the mode is not known, but does not necessarily imply that there was an error or problem with the data. It often means that the information is not available or could not be determined but is expected to be a valid state to encounter.

Should we keep both Invalid and unknown or just use one?

[Abby-Wheelis]

I think we can keep both, thank you for the explanation! Hopefully once we figure out #1088 and resolve the underlying data issues there will be less INVALID sense modes

[TeachMeTW]

@shankari if possible please resolve the issue if no further reviews/changes are needed

[JGreenlee]

I agree with the reasoning of using INVALID rather than UNKNOWN

UNKNOWN exists in the data for trips where a mode could not be determined from the sensed information

INVALID is used to mark trips that are missing the ble summary. INVALID is not stored in the data model

[shankari]

LGTM!

nit/future fix: If we are creating a multi-line function anyway, I don't think it needs to be a lambda, we can make it a real function which may be easier to read.

But that is a style preference that I don't feel that strongly about so merging this anyway

[shankari]

@JGreenlee @TeachMeTW I think this will address some of the issues with loading data on older deployments (e.g. ccebike/smart-commute/mm-masscec etc). We should spot check that once I have deployed to production.