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

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

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

KaushikMalapati
Copy link

@KaushikMalapati KaushikMalapati commented Aug 7, 2023

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
Copy link
Member

ZLLentz commented Aug 7, 2023

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

Copy link
Member

@ZLLentz ZLLentz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@ZLLentz
Copy link
Member

ZLLentz commented Aug 7, 2023

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

@ZLLentz ZLLentz merged commit 992eecf into pcdshub:master Aug 7, 2023
8 of 9 checks passed
@KaushikMalapati
Copy link
Author

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
Copy link
Member

ZLLentz commented Aug 8, 2023

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

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