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

PR Insert widget #109

Merged
merged 105 commits into from
Feb 14, 2024
Merged

PR Insert widget #109

merged 105 commits into from
Feb 14, 2024

Conversation

DanicaSTFC
Copy link
Collaborator

@DanicaSTFC DanicaSTFC commented Nov 22, 2023

Closes #103
closes #105
closes #95
Closes issue #107.
Partially resolves 108.

Full summary:

INSERT WIDGET CHANGES

  • Adds insertWidgetToFormLayout to UIFormWidget, FormDockWidget and FormDialog.
  • Changes name of insertWidget to insertWidgetToVerticalLayout in FormDialog.
  • Adds unit tests for insertWidgetToFormLayout.
  • Add unit test for insertWidgetToVerticalLayout.
  • Adds property list_all_widgets to the unit test class FormsCommonTests.
  • Adds method add_every_spanning_widget to the unit test class FormsCommonTests.
  • Modifies _addWidget so it invokes insertWidgetToFormLayout.
  • Adds example file insert_widgets_example
  • Add getWidgetFromVerticalLayout
  • Add removeWidgetFromVerticalLayout to removewidget in the form dialog.
  • _addWidget has been removed and substituted by the insert method.

DICTIONARY CHANGES

  • Adds popWidgetFromDictionary.
  • Adds getWidgetRow.
  • Change num_widgets to be a property
  • the default state widget dictionary is now initialised in init.

STATE SAVING CHANGES

  • Changes getWidgetState so it reads the widget row too.
  • Changes applyWidgetStateso it sets the widget row.
  • Adds getNameAndRoleFromNameKey and _getNameAndRoleFromWidget
  • Changes applyWidgetStates so it checks states are the same as in the form.
  • Update unit tests for getWidget and applyWidget and saveWidgets etc to incorporate the changes in the state format, which includes the widget row.
  • Add widget number to _label saved state.

OTHER CHANGES

  • Improve some methods docstrings to be clearer.
  • changes capitals in setDefaultWidgetStatesVisibleTrue
  • Add add_every_spanning_widget() in the setup in the tests for all the forms.
  • Add test test_get_name_and_role_from_key_or_widget.
  • The remove/insert widgets is out from the ApplyWidgetStates method and included in the new method adaptFormToStates. This is not included in this PR and will be saved in a separate branch. Its unit test has also been removed.

Before first review.

INSERT WIDGET CHANGES

  • Changes name of insertWidget to insertWidgetToVerticalLayout in FormDialog.
  • Add unit test for insertWidgetToVerticalLayout.
  • Adds insertWidgetToFormLayout to UIFormWidget, FormDockWidget and FormDialog.
  • Adds unit tests for insertWidgetToFormLayout.
  • Adds property list_all_widgets to the unit test class FormsCommonTests.
  • Adds method add_every_spanning_widget to the unit test class FormsCommonTests.
  • Modifies _addWidget so it invokes insertWidgetToFormLayout.
  • Adds example file insert_widgets_example

DICTIONARY CHANGES

  • Adds increaseNumWidgets and decreaseNumWidgets.
  • Adds populateWidgetDictionary.
  • Adds removeWidgetFromDictionary.
  • Modifies removeWidget so that it creates and populates self.removed_widget_dictionary.
  • Adds getRemovedWidgets
  • Separates the generation/population of the dictionaries from the method _addWidget.
  • Adds getWidgetNumber.
  • Adds _populateWidgetNumberDictionary and _popWidgetNumberDictionary.

STATE SAVING CHANGES

  • Changes getWidgetState so it reads the widget number too.
  • Changes applyWidgetStateso it sets the widget number.
  • Adds getNameAndRoleFromNameKey and _getNameAndRoleFromWidget
  • Changes applyWidgetStates so it removes/inserts the widgets in the form in accordance with the widgets present in the state
  • Update unit tests for getWidget and applyWidget and saveWidgets etc to incorporate the changes in the state format, which includes the widget number.

OTHER CHANGES

  • Changed docstrings so they are not repeated for the different forms and we can modify them in one place consistently.
  • Improve some methods docstrings to be clearer.
  • changes capitals in setDefaultWidgetStatesVisibleTrue

@DanicaSTFC DanicaSTFC added this to the v1.0.0 milestone Nov 22, 2023
@DanicaSTFC
Copy link
Collaborator Author

@casperdcl Thank you for your commits. When I pulled them 128 tests failed and 8 passed. I do not see a red cross in the checks above. Could you look into why the checks have passed in GitHub? Also, would you like to fix the code or should I do it?

@@ -23,6 +21,13 @@ class UIFormWidget:
| |
+----------------------------------------------------------+
'''
def __init__(self, *args, **kwargs):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@casperdcl why do we add the init here with the default states? We add the other dictionary in the class below.

@casperdcl
Copy link
Member

casperdcl commented Feb 1, 2024

128 tests failed and 8 passed. I do not see a red cross in the checks above. Could you look into why the checks have passed in GitHub? Also, would you like to fix the code or should I do it?

Please do push any fixes! It might help me find the problem(s) with the tests :/

@DanicaSTFC
Copy link
Collaborator Author

128 tests failed and 8 passed. I do not see a red cross in the checks above. Could you look into why the checks have passed in GitHub? Also, would you like to fix the code or should I do it?

Please do push any fixes! It might help me find the problem(s) with the tests :/

test error

I fixed most errors.
The screenshot shows a a recurring error in the tests. If I delete the folder they run again.

@casperdcl casperdcl mentioned this pull request Feb 1, 2024
@casperdcl
Copy link
Member

casperdcl commented Feb 1, 2024

I fixed most errors. The screenshot shows a a recurring error in the tests. If I delete the folder they run again.

unrelated #128 lol. Issue number 128 for 128 failing tests :D

TL;DR you probably had 1 failing test which didn't clean up after itself so it causes the rest to fail.

DanicaSTFC and others added 2 commits February 1, 2024 15:39
@DanicaSTFC DanicaSTFC requested a review from casperdcl February 1, 2024 18:14
@DanicaSTFC
Copy link
Collaborator Author

@casperdcl Regarding the style of the docstring I am trying to use the CIL convention (with the spaces around ":"). Ideally we would change all of them to be consistent.

@casperdcl
Copy link
Member

spaces around ":"

ah ok sure https://numpydoc.readthedocs.io/en/latest/format.html#parameters no problem.

eqt/ui/UIFormWidget.py Outdated Show resolved Hide resolved
Copy link
Member

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

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

🥳

@casperdcl casperdcl merged commit 558ccab into main Feb 14, 2024
6 checks passed
@casperdcl casperdcl deleted the insert-widget-branch branch February 14, 2024 12:30
@casperdcl casperdcl mentioned this pull request Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants