DataProfiler icon indicating copy to clipboard operation
DataProfiler copied to clipboard

Replace snappy with cramjam

Open gliptak opened this issue 1 year ago • 4 comments

gliptak avatar Feb 02 '24 20:02 gliptak

@taylorfturner please assign reviewers

gliptak avatar Feb 05 '24 21:02 gliptak

Thanks for your patience @gliptak -- we will review your proposal

taylorfturner avatar Feb 05 '24 21:02 taylorfturner

please review

gliptak avatar Feb 20 '24 16:02 gliptak

https://github.com/capitalone/synthetic-data/pull/346

gliptak avatar Feb 26 '24 15:02 gliptak

@gliptak rebase onto dev and I'll approve

taylorfturner avatar Mar 07 '24 12:03 taylorfturner

rebase or update branch on this and I'll approve @gliptak

taylorfturner avatar Mar 07 '24 19:03 taylorfturner

green for your review @taylorfturner

gliptak avatar Mar 07 '24 21:03 gliptak

@micdavis @ksneab7 please review/approve

gliptak avatar Mar 11 '24 14:03 gliptak

@gliptak are you able to restore your cramjam1 branch?

This change fails tests locally on this command DATAPROFILER_SEED=0 python3 -m unittest discover -p "test*.py"

First issues is dataprofiler/dataprofiler/tests/test_dp_logging.py", line 27, in test_default_verbosity self.assertEqual( AssertionError: 20 != 30

Second issue is dataprofiler/dataprofiler/tests/test_data_profiler.py", line 32, in test_set_seed self.assertEqual(dp.settings._seed, None) AssertionError: 0 != None

main testing works locally and the only diff between main and dev is this PR

We can either:

  • restore your branch and revert this PR
  • or I or you can create a new PR to revert back to snappy, as it was prior to this PR

taylorfturner avatar Mar 22 '24 18:03 taylorfturner

Checking out f8b3e5dbd4b76f0ecc291911ace9e8e21cf1ecb1 and running tests on that commit (one commit prior to this PR) ensures that tests did run on dev locally at that point in the history @gliptak

taylorfturner avatar Mar 22 '24 18:03 taylorfturner

Glad to ultimately do cramjam + python 3.11 -- these tests will just need to pass locally as well as remote

taylorfturner avatar Mar 22 '24 19:03 taylorfturner

@taylorfturner restored branch

as the build was green for this PR, might that indicate that different library versions where used?

gliptak avatar Mar 22 '24 19:03 gliptak

potentially -- I tried a bunch of different scenarios and wasn't able to find a setup where the above two tests passed locally.

Did you run DATAPROFILER_SEED=0 python3 -m unittest discover -p "test*.py" on your end locally with cramjam1?

taylorfturner avatar Mar 22 '24 19:03 taylorfturner

I did reproduce the fail locally and still cannot reproduce in GHA

the one difference I observed is that local is Python 3.10.12 while GHA is CPython (3.10.13)

gliptak avatar Mar 24 '24 19:03 gliptak