Skip to content

Ensure datetime_format warning is only raised once #2737

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

Jump to bottom
Merged
fealho merged 7 commits into from
Nov 20, 2025
Merged

Ensure datetime_format warning is only raised once #2737

fealho merged 7 commits into from
Nov 20, 2025

Conversation

@fealho
Copy link
Member

@fealho fealho commented Nov 7, 2025 •
edited
Loading

@sdv-team
Copy link
Contributor

sdv-team commented Nov 7, 2025

Task linked: CU-86b79j1b3 SDV-Enterprise - Warning raised multiple times when metadata does not have the 'datetime_format' key #1569

@codecov
Copy link

codecov bot commented Nov 7, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.10%. Comparing base (526cd12) to head (19bd9b7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2737   +/-   ##
=======================================
  Coverage   98.10%   98.10%           
=======================================
  Files          72       72           
  Lines        8015     8025   +10     
=======================================
+ Hits         7863     7873   +10     
  Misses        152      152
Flag Coverage Δ
integration 75.66% <100.00%> (-0.01%) ⬇️
unit 96.82% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sdv-team
Copy link
Contributor

sdv-team commented Nov 7, 2025

This Pull Request is not linked to an issue. To ensure our community is able to accurately track resolved issues, please link any issue that will be closed by this PR!

@fealho fealho marked this pull request as ready for review November 12, 2025 17:37
@fealho fealho requested a review from a team as a code owner November 12, 2025 17:37
@fealho fealho requested review from amontanez24 and removed request for a team November 12, 2025 17:37
@fealho fealho requested review from R-Palazzo and rwedge and removed request for amontanez24 November 12, 2025 17:37
R-Palazzo
R-Palazzo reviewed Nov 12, 2025
View reviewed changes
@fealho fealho requested a review from R-Palazzo November 12, 2025 18:11
@rwedge
Copy link
Contributor

rwedge commented Nov 12, 2025

Can you move the issue from Enterprise to the SDV repo so our release process will have an issue to associate this PR with?

@rwedge
Copy link
Contributor

rwedge commented Nov 12, 2025

Is there a regression test we could write for this change?

@fealho fealho force-pushed the issue-1569-datetime-warning branch from 5e4b5e8 to 2855e40 Compare November 17, 2025 14:40
amontanez24
amontanez24 requested changes Nov 19, 2025
View reviewed changes
sdv/metadata/single_table.py
A warning is being raised if ``datetime_format`` is missing from a column represented
as ``object`` in the dataframe and its sdtype is ``datetime``.
"""
_datetime_format_warning_flag = sdtype_warnings is not None
Copy link
Contributor

@amontanez24 amontanez24 Nov 19, 2025

Choose a reason for hiding this comment

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

isn't it possible for other warnings to show up in this list? Should we filter through the sdtype_warnings and specifically find ones with datetime in them

tests/integration/multi_table/test_hma.py Outdated
comp_synth.sample(1)

# Assert
assert len(w) == 1
Copy link
Contributor

@amontanez24 amontanez24 Nov 19, 2025

Choose a reason for hiding this comment

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

I think you should filter through the warnings here as well. Sometimes new deprecation warnings show up and tests like this will fail. We should look specifically for the datetime warning

@fealho fealho force-pushed the issue-1569-datetime-warning branch from 1be4b03 to 19bd9b7 Compare November 19, 2025 20:07
@fealho fealho merged commit 37527fe into main Nov 20, 2025
40 of 41 checks passed
@fealho fealho deleted the issue-1569-datetime-warning branch November 20, 2025 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rwedge rwedge rwedge approved these changes

@amontanez24 amontanez24 amontanez24 approved these changes

@R-Palazzo R-Palazzo R-Palazzo approved these changes

Assignees

@fealho fealho

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Warning raised multiple times when metadata does not have the 'datetime_format' key

6 participants

@fealho @sdv-team @rwedge @amontanez24 @R-Palazzo