-
Notifications
You must be signed in to change notification settings - Fork 204
Fix float("inf") and float("nan") JSON serialization breaking reports
#1964
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
Conversation
WalkthroughA utility function was introduced to convert infinite and NaN float values to strings within data structures. This function is now used during report generation to ensure proper JSON serialization. Additionally, a unit test was added to demonstrate the function's behavior with various float values. Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
👋 @clementsicard |
float("inf") and float("nan") JSON serializationfloat("inf") and float("nan") JSON serialization breaking reports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
elementary/utils/json_utils.py (1)
84-84: Fix typo in docstring.The docstring has a typo: "for" should be "and".
- """Replaces occurrences of float("nan") for float("infinity") in the given dict object.""" + """Replaces occurrences of float("nan") and float("infinity") in the given object."""Note: Also consider updating "dict object" to just "object" since the function handles more than just dictionaries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
elementary/monitor/data_monitoring/report/data_monitoring_report.py(2 hunks)elementary/utils/json_utils.py(2 hunks)tests/unit/utils/test_dicts.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/unit/utils/test_dicts.py (1)
elementary/utils/json_utils.py (1)
inf_and_nan_to_str(83-97)
elementary/monitor/data_monitoring/report/data_monitoring_report.py (1)
elementary/utils/json_utils.py (1)
inf_and_nan_to_str(83-97)
🔇 Additional comments (3)
elementary/utils/json_utils.py (1)
83-97: Well-implemented utility function with correct logic.The function properly handles the recursive traversal of data structures and correctly identifies and converts infinity and NaN values to their string representations. The use of numpy's
isinf()andisnan()functions ensures reliable detection of these special float values.elementary/monitor/data_monitoring/report/data_monitoring_report.py (2)
24-24: Proper import addition for the utility function.The import is correctly placed and follows the existing import organization.
78-78: Excellent integration of the utility function.The placement of
json_utils.inf_and_nan_to_str(output_data)before JSON serialization is perfect. This ensures that any infinity or NaN values in the report data are converted to strings, preventing the JSON serialization issues described in the PR objectives.
|
Thanks @clementsicard! Merging :) |
Description of the problem
Infinityvalues (possible in BigQuery, this is equivalent tofloat("inf"), then the generated output when serializing the fetched report data to JSON, then dumping it toelementary_output.jsoncontainsInfinityvalues, sinceallow_nanflag tojson.dumpsisTrueby default (see here).Infinityin the JSON, as it doesn't comply with the JSON specificationResolution
inf_and_nan_to_str, so thatfloat("inf")gets turned into"Infinity"andfloat("nan")into"NaN"as ajson_utilsmethod (hopefully that is an appropriate location).json.dumpscall.Note
Note that we couldn't simply use a
default=orcls=method in the dumps arg, something likebecause the
jsonmodules intercepts these values and turns them into unquoted strings before the custom encoder can be applied...Summary by CodeRabbit
New Features
Tests