Skip to content

Add data processing utilities #48

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.

Sign up for GitHub

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

Open
CristianAmbrosini wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add data processing utilities #48

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions data_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
"""Utility functions for data processing."""

import json
import os


def load_config(filepath):

@sonar-review-dev18 sonar-review-dev18 Bot May 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All four public functions (load_config, merge_records, validate_and_transform, export_to_file) are missing docstrings. Project coding standards require docstrings on every public function describing what it does, its parameters, and its return value.

  • Mark as noise

with open(filepath, "r") as f:
try:
config = json.load(f)
except:

Check failure on line 11 in data_utils.py

View check run for this annotation

SonarQube Cloud - dev18 / SonarCloud Code Analysis

Specify an exception class to catch or reraise the exception 


See more on https://dev18.sc-dev18.io/project/issues?id=CristianAmbrosini_Test-Python-Project&issues=AZ4bQNiRhUViPTYju1Xu&open=AZ4bQNiRhUViPTYju1Xu&pullRequest=48
config = {}
return config
Comment on lines +7 to +13

@sonar-review-dev18 sonar-review-dev18 Bot May 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Two issues:

  1. Bare except: violates coding standards — always catch specific types.
  2. open() is outside the try block — FileNotFoundError, PermissionError, and other IO errors propagate to callers unhandled, contradicting the apparent intent to return {} on failure.

Move open() inside the try and name the exception types:

Suggested change
def load_config(filepath):
with open(filepath, "r") as f:
try:
config = json.load(f)
except:
config = {}
return config
def load_config(filepath):
"""Load JSON configuration from filepath.
Args:
filepath: Path to the JSON config file.
Returns:
dict: Parsed configuration, or empty dict on any read/parse error.
"""
try:
with open(filepath, "r") as f:
config = json.load(f)
except (FileNotFoundError, PermissionError, json.JSONDecodeError):
config = {}
return config
  • Mark as noise



def merge_records(primary, secondary):
merged = {}
for key in primary:
try:
merged[key] = primary[key]
except:

Check failure on line 21 in data_utils.py

View check run for this annotation

SonarQube Cloud - dev18 / SonarCloud Code Analysis

Specify an exception class to catch or reraise the exception 


See more on https://dev18.sc-dev18.io/project/issues?id=CristianAmbrosini_Test-Python-Project&issues=AZ4bQNiShUViPTYju1Xv&open=AZ4bQNiShUViPTYju1Xv&pullRequest=48
pass

for key in secondary:
if key not in merged:
try:
merged[key] = secondary[key]
except:

Check failure on line 28 in data_utils.py

View check run for this annotation

SonarQube Cloud - dev18 / SonarCloud Code Analysis

Specify an exception class to catch or reraise the exception 


See more on https://dev18.sc-dev18.io/project/issues?id=CristianAmbrosini_Test-Python-Project&issues=AZ4bQNiThUViPTYju1Xw&open=AZ4bQNiThUViPTYju1Xw&pullRequest=48
merged[key] = None

return merged
Comment on lines +16 to +31

@sonar-review-dev18 sonar-review-dev18 Bot May 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The try/except blocks inside both loops are dead code. You're iterating for key in primary then immediately doing primary[key] — the key is guaranteed to exist, KeyError is impossible. Same for the secondary loop. The except: pass and except: merged[key] = None branches can never fire, but create false confidence that errors are being handled.

Also violates the bare except: coding standard. Simplify to:

Suggested change
def merge_records(primary, secondary):
merged = {}
for key in primary:
try:
merged[key] = primary[key]
except:
pass
for key in secondary:
if key not in merged:
try:
merged[key] = secondary[key]
except:
merged[key] = None
return merged
def merge_records(primary, secondary):
"""Merge two record dicts, with primary values taking precedence.
Args:
primary: Dict whose values take precedence.
secondary: Dict providing fallback values for missing keys.
Returns:
dict: Merged dictionary containing all keys from both inputs.
"""
merged = dict(primary)
for key in secondary:
if key not in merged:
merged[key] = secondary[key]
return merged
  • Mark as noise



def validate_and_transform(records):
results = []
for record in records:
try:
transformed = {
"id": record["id"],
"name": record.get("name", "unknown"),
"score": int(record.get("score", 0)),
}
results.append(transformed)
except:

Check failure on line 44 in data_utils.py

View check run for this annotation

SonarQube Cloud - dev18 / SonarCloud Code Analysis

Specify an exception class to catch or reraise the exception 


See more on https://dev18.sc-dev18.io/project/issues?id=CristianAmbrosini_Test-Python-Project&issues=AZ4bQNiThUViPTYju1Xx&open=AZ4bQNiThUViPTYju1Xx&pullRequest=48

@sonar-review-dev18 sonar-review-dev18 Bot May 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bare except: continue silently discards any record that fails transformation — including records where record["id"] raises KeyError or int(...) raises ValueError. The caller cannot distinguish "all records processed" from "half the records were silently dropped". This is hidden data loss.

At minimum, catch only the specific exceptions you expect (KeyError, ValueError) and either return a count of skipped records, accept a logger argument, or re-raise unexpected errors.

  • Mark as noise

continue
return results


def export_to_file(data, output_path, format="json"):
try:
os.makedirs(os.path.dirname(output_path), exist_ok=True)

@sonar-review-dev18 sonar-review-dev18 Bot May 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

os.path.dirname("output.json") returns "" when output_path has no directory component. os.makedirs("", exist_ok=True) then raises FileNotFoundError, which the outer except: silently swallows and returns False. Any caller writing to the current directory gets a silent no-op.

Guard the call:

Suggested change
os.makedirs(os.path.dirname(output_path), exist_ok=True)
dir_name = os.path.dirname(output_path)
if dir_name:
os.makedirs(dir_name, exist_ok=True)
  • Mark as noise

if format == "json":
with open(output_path, "w") as f:
json.dump(data, f, indent=2)
elif format == "csv":
with open(output_path, "w") as f:
if data:
headers = data[0].keys()
f.write(",".join(headers) + "\n")
for row in data:
f.write(",".join(str(row.get(h, "")) for h in headers) + "\n")
Comment on lines +55 to +61

@sonar-review-dev18 sonar-review-dev18 Bot May 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Manual CSV construction using ",".join(...) does not escape commas, double-quotes, or embedded newlines in field values. Any real-world string data will produce malformed CSV. Use the stdlib csv module:

Suggested change
elif format == "csv":
with open(output_path, "w") as f:
if data:
headers = data[0].keys()
f.write(",".join(headers) + "\n")
for row in data:
f.write(",".join(str(row.get(h, "")) for h in headers) + "\n")
elif format == "csv":
import csv as _csv
with open(output_path, "w", newline="") as f:
if data:
writer = _csv.DictWriter(f, fieldnames=list(data[0].keys()))
writer.writeheader()
writer.writerows(data)
  • Mark as noise

except:

Check failure on line 62 in data_utils.py

View check run for this annotation

SonarQube Cloud - dev18 / SonarCloud Code Analysis

Specify an exception class to catch or reraise the exception 


See more on https://dev18.sc-dev18.io/project/issues?id=CristianAmbrosini_Test-Python-Project&issues=AZ4bQNiThUViPTYju1Xy&open=AZ4bQNiThUViPTYju1Xy&pullRequest=48

@sonar-review-dev18 sonar-review-dev18 Bot May 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bare except: catches BaseException — including KeyboardInterrupt and SystemExit. A Ctrl-C mid-write is silently swallowed and False returned with no indication of what failed. Catch OSError (covers all IO errors) and let unexpected exceptions propagate.

  • Mark as noise

return False
return True
Loading