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

MeasurementSetXds has a .sel() method that shadows Dataset.sel() with a different interface #293

Open
v-morello opened this issue Oct 18, 2024 · 2 comments

Comments

@v-morello
Copy link

Helllo there,

I upgraded to v0.0.42 today, and I was surprised that the following code snippet does not work anymore on a MeasurementSetXds:

xds.sel({"polarization": ["XX", "YY"]})

yields:

     67 if data_group_name is not None:
     68     sel_data_group_set = set(
---> 69         self.attrs["data_groups"][data_group_name].values()
     70     )
     72     data_variables_to_drop = []
     73     for dg in self.attrs["data_groups"].values():

TypeError: unhashable type: 'dict'

The reason is that I think I am calling Dataset.sel, but xradio forces me to call MeasurementSetXds.sel() which has a different interface and use case:

def sel(self, data_group_name=None, **kwargs):

I assume this is accidental, but anyway this method needs to have a different name, overriding any of the fundamental Dataset methods is not something we can afford in general.

@v-morello v-morello changed the title MeasurementSetXds has a .sel() method that shadows Dataset.sel() with a different interface, making the original one impossible to use MeasurementSetXds has a .sel() method that shadows Dataset.sel() with a different interface Oct 18, 2024
@v-morello
Copy link
Author

I had a look at the code and the core problem is that MeasurementSetXds.sel() does two things instead of one:

  1. Optionally, keep only the data variables from the data group you want to select, data group being a concept specific to xradio.
  2. Call Dataset.sel()

Suggested solution: get rid of that specialized sel(), instead there should be a select_data_group() method that does only that, and that can be chained independently with Dataset.sel(). Both of these calls should then work:

xds.select_data_group("corrected").sel({"polarization": ["XX", "YY"]})
xds.sel({"polarization": ["XX", "YY"]}).select_data_group("corrected")

@v-morello
Copy link
Author

@Jan-Willem Is this fixed in more recent releases?

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

No branches or pull requests

1 participant