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

[FIX] Make widgets PyQt6 compatible #929

Merged
merged 3 commits into from
Jan 30, 2023

Conversation

PrimozGodec
Copy link
Collaborator

@PrimozGodec PrimozGodec commented Jan 17, 2023

Issue

Keywords, Preprocess Text and Score Documents widgets are not fully PyQt6 compatible

Description of changes

Make the widget PyQt6 compatible.
Comment regarding storing sorting order to settings. As discussed at the meeting, the sort order should not be held as Qt.SortOrder (since it is a Qt-specific object). It must be saved as an integer. It is later transformed into Qt.SortOrder when needed. PyQt5's SortOrder is an IntEnum-like object, while in PyQt6, it is Enum.

Includes
  • Code changes
  • Tests
  • Documentation

@PrimozGodec PrimozGodec changed the title [FIX] Make remaining widgets PyQt6 compatible [FIX] Make widgets PyQt6 compatible Jan 17, 2023
@janezd janezd assigned noahnovsak and unassigned VesnaT Jan 27, 2023
Comment on lines 138 to 146
def validate(self, *args):
# accept empty input
valid, text, pos = super().validate(*args)
return 2 if valid else 0, text, pos
new_state = (
QValidator.State.Acceptable
if valid == QValidator.State.Acceptable
else QValidator.State.Invalid
)
return new_state, text, pos

def valueFromText(self, text: str) -> int:
return max(int(text) if text else self.minimum(), self.minimum())
Copy link
Contributor

Choose a reason for hiding this comment

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

When writing this, the intended behaviour was, that the user could delete everything in the upper limit spinbox (or write some smaller value), and it would default to it's minimum (interpreted as no upper bound). Now it won't allow for that. I feel like the behaviour has changed with Qt6. But I could have just written it poorly in the first place. At any rate, I would either delete both these functions completely, if the current behaviour is okay, or find a way to allow for the aforementioned behaviour.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@noahnovsak, thank you for noticing this one. It was my mistake. I wrongly changed the condition 2 if valid else 0 to QValidator.State.Acceptable if valid == QValidator.State.Acceptable else QValidator.State.Invalid, which is indeed not the same. I think now it should work.

@codecov-commenter
Copy link

Codecov Report

Merging #929 (9bd8567) into master (25952ea) will increase coverage by 0.08%.
The diff coverage is 87.50%.

❗ Current head 9bd8567 differs from pull request most recent head 484fe58. Consider uploading reports for the commit 484fe58 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #929      +/-   ##
==========================================
+ Coverage   77.53%   77.61%   +0.08%     
==========================================
  Files          86       86              
  Lines       12261    12274      +13     
  Branches     1608     1608              
==========================================
+ Hits         9507     9527      +20     
+ Misses       2455     2449       -6     
+ Partials      299      298       -1     

@noahnovsak noahnovsak merged commit a9c89a6 into biolab:master Jan 30, 2023
@PrimozGodec PrimozGodec deleted the pyqt6-compat branch February 28, 2023 13: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