Skip to content

Commit 9fb6905

Browse files
kbighorseclaude
andcommitted
fix: address second round of Copilot review comments
- Fix date→datetime promotion: use isinstance checks instead of hasattr(tzinfo) which doesn't work for plain date objects - Use param.default_unit for unit fallback instead of hardcoded ug/L - Parse BDD AnalysisDate as date (not datetime) to match DB column type - Guard after_scenario cleanup to skip DB session when no fixtures created Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 2d6cd6e commit 9fb6905

3 files changed

Lines changed: 16 additions & 16 deletions

File tree

tests/features/environment.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -768,10 +768,10 @@ def after_scenario(context, scenario):
768768

769769
# Chemistry backfill cleanup — runs regardless of DROP_AND_REBUILD_DB
770770
# because the backfill steps create their own fixture data.
771-
if getattr(context, "_backfill_created", None) is not None:
771+
created = getattr(context, "_backfill_created", None)
772+
if created is not None and any(created.values()):
772773
try:
773774
with session_ctx() as session:
774-
created = context._backfill_created
775775

776776
# Delete in FK order: Notes → Observations → Samples → FieldActivities → FieldEvents → NMA rows
777777
# Scope to observations linked to this scenario's samples

tests/features/steps/chemistry-backfill.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -822,12 +822,12 @@ def _create_minor_trace_from_fields(context: Context, fields: dict):
822822
session.flush()
823823
context._backfill_created["nma_sampleinfo_ids"].append(sampleinfo.id)
824824

825-
# Parse analysis date
825+
# Parse analysis date as a plain date to match the Date column type
826826
analysis_date = None
827827
if fields.get("AnalysisDate"):
828828
analysis_date = datetime.strptime(
829829
fields["AnalysisDate"], "%Y-%m-%d"
830-
).replace(tzinfo=timezone.utc)
830+
).date()
831831

832832
# Parse numeric fields
833833
sample_value = (

transfers/backfill/chemistry_backfill.py

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -398,30 +398,30 @@ def _backfill_minor_trace_chemistry_impl(session: Session) -> BackfillResult:
398398
# Build observation_datetime — observation_datetime is NOT NULL in the DB,
399399
# so rows without AnalysisDate must be skipped.
400400
obs_dt = row.analysis_date
401-
if obs_dt is not None:
402-
if hasattr(obs_dt, "tzinfo") and obs_dt.tzinfo is None:
403-
from datetime import datetime, time
404-
405-
if isinstance(obs_dt, datetime):
406-
obs_dt = obs_dt.replace(tzinfo=timezone.utc)
407-
else:
408-
# date object — promote to datetime at midnight UTC
409-
obs_dt = datetime.combine(obs_dt, time.min, tzinfo=timezone.utc)
410-
else:
401+
if obs_dt is None:
411402
result.errors.append(
412403
f"Row GlobalID={global_id_str} has no AnalysisDate — skipping"
413404
)
414405
continue
415406

416-
# unit is NOT NULL in the DB — fall back to parameter default_unit
417-
unit = row.units if row.units else "ug/L"
407+
from datetime import date as date_type, datetime, time
408+
409+
if isinstance(obs_dt, datetime):
410+
if obs_dt.tzinfo is None:
411+
obs_dt = obs_dt.replace(tzinfo=timezone.utc)
412+
elif isinstance(obs_dt, date_type):
413+
obs_dt = datetime.combine(obs_dt, time.min, tzinfo=timezone.utc)
414+
# else: already timezone-aware datetime — use as-is
418415

419416
savepoint = session.begin_nested()
420417
try:
421418
param = _get_or_create_parameter(
422419
session, analyte, "water", default_unit="ug/L"
423420
)
424421

422+
# unit is NOT NULL in the DB — fall back to parameter's default_unit
423+
unit = row.units if row.units else param.default_unit
424+
425425
analysis_method_id = None
426426
if row.analysis_method:
427427
am = _get_or_create_analysis_method(session, row.analysis_method)

0 commit comments

Comments
 (0)