pcdshub / pcdswidgets

LCLS PyDM Widget Library
https://pcdshub.github.io/pcdswidgets
Other
0 stars 10 forks source link

BUG fixing activate_filter by rearranging parameters to work with qtpy #85

Closed KaushikMalapati closed 1 year ago

KaushikMalapati commented 1 year ago

Description

One line change, switched order of active and filter_name paramters in activate_filter from table.py.

Motivation and Context

Currently, attempting to change the filter results in: TypeError: activate_filter() got multiple values for argument 'filter_name', presumably because filter_name is the first argument and is given by a keyword argument in the slot for the filter menu, but the pyqt gives the bool as the first positional argument, causing there to be multiple values for filter_name and none for active.

I think this is caused by the use of functools partial: the signal calls the partial object with one argument, but this is given as the first positional argument, which was already given in the creation of the object as a keyword argument.

How Has This Been Tested?

I made the change in my local repo and used pydm to create a window with a FilterSortWidgetTable. After making the change, using the filter menu worked as expected, but before it caused the above TypeError. I feel this is sufficient since it's a one-line edit and activate_filter is only used in one place.

Where Has This Been Documented?

I didn't change any documentation (the docstring already has the parameters in the "right" order)

ZLLentz commented 1 year ago

Good catch I've seen something similar before where different versions of pyqt would send the arg positionally or by keyword...

ZLLentz commented 1 year ago

The CI failure is unrelated, I'll make an issue for it

KaushikMalapati commented 1 year ago

It is at least a bit confusing to me why the partial call here wasn't sufficient but I'm on board with the order change

from functools import partial
def a(b, c):
    print(b, c)
d = partial(a, b='Hello,')
d('World!')

This results in TypeError: a() got multiple values for argument 'b' whereas

d = partial(a, c='World!')
d('Hello,')

results in Hello, World!

It seems that partial will always pass arguments positionally, even if they are already passed as keyword arguments.

ZLLentz commented 1 year ago

Ah I see, it's the mixing that does us in here. I'll be more careful with this.