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

Reinstate support for single correlation measurement sets. #449

Merged
merged 3 commits into from
Mar 24, 2022

Conversation

JSKenyon
Copy link
Collaborator

@JSKenyon JSKenyon commented May 7, 2021

What it says on the label. Use at your own risk as things may break. @o-smirnov I distinctly remember writing the code to handle single correlation measurement sets inside _column_to_cube but it seems at some point we disabled it at a higher level. Any idea why? This runs through on my test data using both the predict and a model column. Predict may be a bit dodgy if the field has polarisation, but this is not going to be a mainstream mode. @KshitijT feel free to give this a shot.

@JSKenyon JSKenyon requested a review from o-smirnov May 7, 2021 07:53
@KshitijT
Copy link
Collaborator

KshitijT commented May 7, 2021

Thanks @JSKenyon !

@bennahugo
Copy link
Collaborator

bennahugo commented Aug 10, 2021 via email

@o-smirnov
Copy link
Collaborator

Put a big red error message in there and leave it at that? Facet-based predict + single-corr MSs is a very obscure corner of the parameter space which I'm not too worried about overlooking...

@bennahugo
Copy link
Collaborator

bennahugo commented Aug 10, 2021 via email

@JSKenyon
Copy link
Collaborator Author

This was never going to be merged until someone played around with it. Perfectly happy for it to remain as a PR until someone battle-tests it as this is not a very common use-case.

@bennahugo
Copy link
Collaborator

@KshitijT can you confirm that this works so I can push a release with it (or without it)?

@bennahugo bennahugo merged commit 59f2859 into master Mar 24, 2022
@bennahugo bennahugo deleted the single_corr branch March 24, 2022 09:57
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.

4 participants