Skip to content
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

Raise a VersionError when re-fitting a synthesizer that was created on a previous SDV version #1886

Conversation

pvk-developer
Copy link
Member

Resolves #1838
CU-86azkp7m4

@pvk-developer pvk-developer marked this pull request as ready for review April 3, 2024 16:36
@pvk-developer pvk-developer requested a review from a team as a code owner April 3, 2024 16:36
@pvk-developer pvk-developer requested review from gsheni, frances-h and R-Palazzo and removed request for a team and gsheni April 3, 2024 16:36
@codecov-commenter
Copy link

codecov-commenter commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.47%. Comparing base (df7da68) to head (5c34996).
Report is 2 commits behind head on main.

❗ Current head 5c34996 differs from pull request most recent head 1980b4e. Consider uploading reports for the commit 1980b4e to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1886      +/-   ##
==========================================
- Coverage   97.51%   97.47%   -0.05%     
==========================================
  Files          51       51              
  Lines        5119     4985     -134     
==========================================
- Hits         4992     4859     -133     
+ Misses        127      126       -1     

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

@pvk-developer pvk-developer changed the title Improve version checking and use it when fitting Raise a VersionError when re-fitting a synthesizer that was created on a previous SDV version Apr 3, 2024
Copy link
Contributor

@R-Palazzo R-Palazzo left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@amontanez24, @npatki The current solution will not be applied to DayZ. When updating DayZ, we may want to fix this as well.

tests/integration/multi_table/test_hma.py Show resolved Hide resolved
@pvk-developer pvk-developer force-pushed the issue-1838-show-an-error-when-attempting-to-refit-a-synthesizer-from-previous-sdv branch from e792bb2 to bc431b9 Compare April 8, 2024 16:29
@pvk-developer
Copy link
Member Author

LGTM! 👍

@amontanez24, @npatki The current solution will not be applied to DayZ. When updating DayZ, we may want to fix this as well.

Bear in mind that DayZ does not fit.

sdv/_utils.py Show resolved Hide resolved
sdv/_utils.py Show resolved Hide resolved
sdv/_utils.py Outdated Show resolved Hide resolved
@pvk-developer pvk-developer force-pushed the issue-1838-show-an-error-when-attempting-to-refit-a-synthesizer-from-previous-sdv branch from bc431b9 to 1980b4e Compare April 10, 2024 16:10
@pvk-developer pvk-developer merged commit c4e8540 into main Apr 11, 2024
37 checks passed
@pvk-developer pvk-developer deleted the issue-1838-show-an-error-when-attempting-to-refit-a-synthesizer-from-previous-sdv branch April 11, 2024 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show an error when attempting to re-train a synthesizer that was created on a previous SDV version
6 participants