-
Notifications
You must be signed in to change notification settings - Fork 38
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
feat: add support for flag enum #207
Conversation
WalkthroughThe changes primarily enhance the handling of enum classes in the Changes
TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Files selected for processing (2)
- src/superqt/combobox/_enum_combobox.py (4 hunks)
- tests/test_enum_comb_box.py (3 hunks)
Files skipped from review due to trivial changes (1)
- tests/test_enum_comb_box.py
Additional comments (Suppressed): 3
src/superqt/combobox/_enum_combobox.py (3)
1-8: The new imports
Flag
,reduce
,combinations
, andor_
are introduced to support the flag enum functionality. Ensure that these modules are used appropriately in the code.27-28: The
_get_name_with_value
function is a helper function that returns a tuple of the human-readable name and the original Enum member. This function is used to create a dictionary mapping from names to Enum members, which is useful for supporting flag enums.88-92: The
currentEnum
method has been updated to retrieve the selected enum member from the dictionary created by_get_name_with_value
instead of directly from the enum class. This change supports the retrieval of combined flag enum members.- else: - return list(self._enum_class.__members__.values())[ - self.currentIndex() - 1 - ] - return list(self._enum_class.__members__.values())[self.currentIndex()] + return self._get_enum_member_list(self._enum_class)[self.currentText()]Ensure that this change does not affect the retrieval of non-flag enum members.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #207 +/- ##
==========================================
+ Coverage 87.34% 87.38% +0.04%
==========================================
Files 45 45
Lines 3326 3346 +20
==========================================
+ Hits 2905 2924 +19
- Misses 421 422 +1
☔ View full report in Codecov by Sentry. |
Thanks @Czaki Looks like it needs updating for everything below 3.11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Files selected for processing (2)
- src/superqt/combobox/_enum_combobox.py (4 hunks)
- tests/test_enum_comb_box.py (3 hunks)
Additional comments (Suppressed): 8
tests/test_enum_comb_box.py (3)
1-5: The new hunk imports additional classes
Flag
andIntFlag
from theenum
module. These are used to define flag enumerations in the test cases.43-78: New flag enumeration classes (
IntFlag1
,Flag1
,IntFlag2
,Flag2
,FlagOrNum
) are defined for testing purposes. The values of these enums are set up in a way that allows for testing various scenarios, such as collisions and combinations of flags.171-277: Several new test cases have been added to verify the behavior of the
QEnumComboBox
class with flag enumerations and string enumerations (introduced in Python 3.11). These tests cover scenarios like creating the combo box with flag enums, handling collisions between enum members, and dealing with different representations of flag combinations in different Python versions. The use ofpytest.mark.skipif
ensures that certain tests are only run under appropriate Python versions.src/superqt/combobox/_enum_combobox.py (5)
1-9: The new imports
Flag
,reduce
,combinations
, andor_
are introduced to support the handling of flag enums. Ensure that these modules are used correctly throughout the code.21-38: The
_get_name
function has been updated to handle Flag enums. It now checks if the enum value is an instance of Flag and if so, it decomposes the flag into its constituent members and joins their names with a '|'. This change seems to be backward compatible as it still handles non-flag enums in the same way as before. However, ensure that this change does not affect any existing functionality where_get_name
is used.41-42: The new helper function
_get_name_with_value
returns a tuple containing the name and value of an enum member. Verify that this function is used correctly in the code.68-84: The
setEnumClass
method has been significantly modified. It now uses the new helper function_get_enum_member_list
to generate a dictionary of enum member names and values. If the enum is a subclass of Flag, it generates all possible combinations of flag members. This change should improve the handling of flag enums, but make sure it doesn't break the handling of regular enums.102-106: The
currentEnum
method has been updated to use the new_get_enum_member_list
function. This change should improve the handling of both regular and flag enums. However, verify that this change does not affect any existing functionality wherecurrentEnum
is used.
Yes. I also hit problems as there was a bug in the new name mechanism in Python 3.11(.3). As it is fixed in 3.11.5 then I left only the workaround code with a docstring explanation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Files selected for processing (2)
- src/superqt/combobox/_enum_combobox.py (4 hunks)
- tests/test_enum_comb_box.py (3 hunks)
Files skipped from review due to trivial changes (1)
- tests/test_enum_comb_box.py
Additional comments (Suppressed): 2
src/superqt/combobox/_enum_combobox.py (2)
1-9: The new imports
Flag
,reduce
,combinations
, andor_
are introduced to support flag enums. Thesys
module is imported to handle version-specific behavior for Python versions below 3.11.111-115: The
currentEnum
method now uses the_get_enum_member_list
function to return the currently selected Enum member. This is a more flexible approach than the previous implementation, which relied on the current index of the combobox. It should work well for both regular and flag enums.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great digging. thanks for the updates and all the great tests! LGTM
As followup #205 I have checked that we do not have a test for another enum base class. So I prevent future failures, I added tests and spotted that flag support was not fully functional as it does not support element combinations.
Also add
StrEnum
test.Summary by CodeRabbit
QEnumComboBox
class to supportFlag
enums in addition to regular enums. This allows users to select multiple options from a combo box.QEnumComboBox
by introducing helper functions for retrieving enum member names and values.Flag
enum support inQEnumComboBox
, ensuring robust functionality and error handling.