Skip to content

py3 port (#1)#2

Open
robertdewitt wants to merge 3 commits intofelixpatzelt:masterfrom
robertdewitt:master
Open

py3 port (#1)#2
robertdewitt wants to merge 3 commits intofelixpatzelt:masterfrom
robertdewitt:master

Conversation

@robertdewitt
Copy link

  • Green test-suite on Python 3: slice & dict fixes

  • Amending tox test conditions

  • adding ci.yaml

  • added full docstrings

  • correting ci.yaml syntax error

  • py3-port: make batch.py import relative, drop Py2 refs, bump to v1.0.3

  • batch: restore calibrate_tim2_day convenience wrapper

  • batch: split concatenated kernel so helper returns (G_pc, G_npc, meta)

  • batch: changelog update

* Green test-suite on Python 3: slice & dict fixes

* Amending tox test conditions

* adding ci.yaml

* added full docstrings

* correting ci.yaml syntax error

* py3-port: make batch.py import relative, drop Py2 refs, bump to v1.0.3

* batch: restore calibrate_tim2_day convenience wrapper

* batch: split concatenated kernel so helper returns (G_pc, G_npc, meta)

* batch: changelog update
@robertdewitt
Copy link
Author

robertdewitt commented Aug 2, 2025

Hi, hopefully this brings this package up to 3.8 compatibility!

@felixpatzelt
Copy link
Owner

Hi @robatobalzac, thank you for the PR. I just had a quick look (using python 3.13 actually) and found some small issues:

  • When running the example notebooks, priceprop/examples/calibrate_batch_demo.ipynb fails with TypeError: 'dict_keys' object is not subscriptable. That one should be easily fixable.
  • Can you test if the examples work on your machine? It is unfortunate the test suite didn't flag this I know.
  • You added an empty file tests/test_tim2.py. Did you forget to add a later non-empty version maybe?

Would you like to add that to your PR?

In addition I noticed a small issue with the scorr package with python >= 3.12 but I can take care of that tomorrow.

@felixpatzelt
Copy link
Owner

Ah one more thing: It looks like we are dropping Python 2 support then, which is fine. I would like to do a major version bump in this case and not just a patch.

@robertdewitt
Copy link
Author

robertdewitt commented Aug 4, 2025 via email

@robertdewitt
Copy link
Author

Hi! the iteration update seems to highlight some other issues.
--> 387 s = samples[i-1]
388 m = masks[s]

Was s meant to be i-1?

samples = list(masks.keys())

for i in range(len(samples)):
    # get calibration for a sample
    ## get a sample name and the corresponding mask
    s = samples[i]
    m = masks[s]
    
    ## calculate now or rely on existing calibration?
    if calibrate:
        cal = calibrate_models(
            tt.loc[m], nfft=nfft, group=group, models=models
        )
        if 'cal' in dbc:
            dbc['cal'][s] = cal
        else:
            dbc['cal'] = {s: cal}
    else:
        cal = dbc['cal'][s]

Even correcting these issues leads to others, any ideas on this one? I'm not yet that familiar with the code and this may be obvious to you. My apologies if not!


AssertionError Traceback (most recent call last)
File :1

File ~/My Drive/MscThesis/Project-Code/priceprop/priceprop/batch.py:392, in calc_models(dbc, nfft, group, calibrate, split_by, rshift, models, smooth_kernel)
390 ## calculate now or rely on existing calibration?
391 if calibrate:
--> 392 cal = calibrate_models(
393 tt.loc[m], nfft=nfft, group=group, models=models
394 )
395 if 'cal' in dbc:
396 dbc['cal'][s] = cal

File ~/My Drive/MscThesis/Project-Code/priceprop/priceprop/batch.py:244, in calibrate_models(tt, nfft, group, models)
242 # triple cross correlations
243 if 'hdim2' in models:
--> 244 res['ccccorr'] = scorr.x3corr_grouped_df(
245 tt, ['change', 'sc', 'sc'], nfft=nfft, **kwargs
246 )[0]
247 res['nnccorr'] = scorr.x3corr_grouped_df(
248 tt, ['change', 'sn', 'sn'], nfft=nfft, **kwargs
249 )[0]
250 res['cnccorr'] = scorr.x3corr_grouped_df(
251 tt, ['change', 'sc', 'sn'], nfft=nfft, **kwargs
252 )[0]

File /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/scorr/corr3.py:304, in x3corr_grouped_df(df, cols, by, nfft, funcs, subtract_mean, norm, debias)
302 zi = funcs2.values - subtract[1]
303 pad = max(nfft - lgs, 0)
--> 304 ci = x3corr(
305 xi, yi, zi,
306 nfft=nfft, pad=pad, subtract_mean=sm, norm=nd, debias=debias
307 )
308 C += ci
309 Ce += ci**2

File /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/scorr/corr3.py:168, in x3corr(x, y, z, nfft, pad, subtract_mean, norm, debias)
166 B = np.zeros((nfft, nfft), dtype=complex)
167 for i in range(nit):
--> 168 B += fft2x(
169 x[ti[i]:ti[i]+ndat],
170 y[ti[i]:ti[i]+ndat],
171 z[ti[i]:ti[i]+ndat],
172 nfft = nfft # zero pad
173 )
175 # normalisation
176 if norm == "cov":

File /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/scorr/corr3.py:24, in fft2x(x, y, z, nfft)
21 y = np.ravel(y)
22 z = np.ravel(z)
---> 24 assert len(x) == len(y) and len(x) == len(z), (
25 "Arrays must have the same length"
26 )
28 if not nfft:
29 # let's hope it's not too much!
30 nfft = len(x)

AssertionError: Arrays must have the same length

@felixpatzelt
Copy link
Owner

I will have a look tomorrow. Today I prepared the PR for the scorr package that makes it work for me under Python 3.13. I found some issues with the notebooks there as well due to a change in pandas.

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.

2 participants