-
Notifications
You must be signed in to change notification settings - Fork 0
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
Class architecture refactoring #22
Conversation
…ass and replaced with a private method in TuPostProcessingGui class
…nd StatusBar classes in gui_widgets module
…ses dealing with the reading of Direct-Access files
…rator' class with a dataclass and its methods with two functions providing the functionalities for building the dataclass and running the plotting executable
…classes with a single one storing all the characteristics and with 'group' attribute being of type 'GroupType'; added a function changing this dataclass default values
…Field' class name into 'LabelledCombobox' to help its understanding
…dsConfigurator' class with a corresponding dataclass and a function configuring and returning an instance of this dataclass
…configuration.py' module
…module Moved 'IDGA' and 'IANT' enums in the newly created 'support' module. These enums now have elements defined as tuples of two values, the first being an index, the second a descriptive string. Two properties have been defined for accessing the elements of the tuple.
Substituted class 'LabelImage' with a function that builds an instance of the 'ttk.Label' class. This function loads and assigns the image to the label and returns the built object.
Replaced 'PliReader' class with a dataclass and its method with a function that read the information from the .pli file and returns a properly configured object of this dataclass, which stores all the extracted information.
…hanges The class 'TabContentBuilder' has been made abstract. Its methods 'set_times' and '_build_configuration_fields' have been defined with the proper decorator, so to be implemented by the derived classes. The arguments of the 'set_times' method have been changed into **kwargs: proper checks have been introduced in order to be sure that the right arguments are passed.
The argument of the 'set_times' method for the 'TuStatTabContentBuilder' class had an incorrect check on the required argument. Now the correct name has been provided.
…dule The 'support.py' module has been added to the repository.
The inner methods of the 'TuInp' dataclass have been substituted with two functions each one returning an object of type 'TuInp' with its attributes appropriately filled according to the 'TuPlot' or the 'TuStat' case.
…itance removal The inheritance from the 'object' class has been removed from the 'WidgetTooltip' class, since useless.
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.
Modifications requested in task 1 have been satisfactorily handled.
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.
Modifications requested in task 2 have been satisfactorily handled.
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.
Modifications requested in task 3 have been satisfactorily handled.
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.
Something to change relating to task 4:
- in function
init_DatGenerator()
, assign the result of the instantiation of the dataclassDatGenerator
to a variable; - call the function
run_plot_files_generation()
by passing the above created variable directly within the functioninit_DatGenerator()
; - rename the function
init_DatGenerator()
asinit_run_DatGenerator()
; - make the function
init_DatGenerator()
return the instance ofDatGenerator
.
This way, the instantiation and run instructions in main.py
are joined, the safety being increased.
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.
Something to change relating to task 5 to improve the readability and the modularity:
- to move the function
define_group_from_num()
as static method into the dataclassDiagramCharacteristics
; - to change the arguments of the function
define_group_from_num()
by passing all the data needed to build and return an instance of the dataclassDiagramCharacteristics
; - to make the function
define_group_from_num()
create from scratch the dataclass to return.
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.
Modifications requested in task 6 have been satisfactorily handled.
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.
Something to change relating to task 7 to improve the readability and the modularity:
- to move the function
init_GuiPlotFieldsConfigurator_attrs()
as static method into the dataclassGuiPlotFieldsConfigurator
; - to move all the functions called by
init_GuiPlotFieldsConfigurator_attrs()
into the dataclassGuiPlotFieldsConfigurator
as private methods.
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.
Modifications requested in task 8 have been satisfactorily handled.
Instead of a constructor, in IDGA
and IANT
two properties have been implemented, that is, index
and description
, which perform better the required task.
tugui/gui_widgets.py
Outdated
""" | ||
Class that provides a label filled with an image. | ||
Method that provides a label filled with an image. It builds an instance |
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.
Please, change "Method" into "Function" since outside a class.
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.
Modifications requested in task 9 have been satisfactorily handled.
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.
Something to change relating to task 10 to improve the readability and the modularity:
- to move the function
init_PliReader()
as static method into the dataclassPliReader
.
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.
Modifications requested in task 11 have been satisfactorily handled.
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.
Something to change relating to task 12 to improve the readability and the modularity:
- to move the function
configure_tuplot_inp_fields()
as static method into the dataclassTuInp
; - to move the function
configure_tustat_inp_fields()
as static method into the dataclassTuInp
;
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.
Modifications requested in task 13 have been satisfactorily handled.
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.
The review identified the following modifications to be applied:
Changes requested from review #22 (review) of task 4 have been managed. A static method has been added to the 'DatGenerator' dataclass that builds and returns an object of it. It calls the function 'run_plot_files_generation()' as well. The name 'init_DatGenerator_and_run_exec()' has been choosen for the static method instead of the one indicated in the review.
Some modifications have been made errouneously in commit deb2d13. Now the version of the master has been restored.
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.
Modifications requested in the review of task 4 have been satisfactorily handled.
Changes requested from review #22 (review) of task 5 have been managed. A static method has been added to the 'DiagramCharacteristics' dataclass that builds and returns an object of it. This method also configures the 'group' attribute according to the plot number value.
Changes requested from review #22 (review) of task 7 have been managed. The function 'init_GuiPlotFieldsConfigurator_attrs()' has been made a static method of the 'GuiPlotFieldsConfigurator' dataclass. This method builds and returns an object of the dataclass. In addition, the previously called functions from this method have been changed into private methods of the 'GuiPlotFieldsConfigurator' dataclass.
Change requested from review comment #22 (comment) has been managed. Changed 'Method' to 'Function' in the docstring for the 'provide_label_image()' function in the 'gui_widgets.py' module.
Changes requested from review #22 (review) have been managed. The function 'init_PliReader()' has been moved as a static method into the 'PliReader' dataclass. An improper instantiation of the 'PliReader' dataclass in the 'save_loaded_inp()' method of the 'InpHandler' class has been corrected as well.
Changes requested from review #22 (review) have been managed. The functions 'configure_tuplot_inp_fields()' and 'configure_tustat_inp_fields()' have been moved as static methods into the 'TuInp' dataclass.
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.
Modifications requested in the review of task 5 have been satisfactorily handled
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.
Modifications requested in the review of task 7 have been satisfactorily handled.
One additional modification is suggested: to change the methods _check_config_file_existence()
, _check_exe_file_existence()
and _build_nVsKn()
into private methods since they are used only in the class factory method
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.
Applied the requested modification to the result of task 8 management.
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.
Modifications requested in the review of task 10 have been satisfactorily handled.
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.
Modifications requested in the review of task 12 have been satisfactorily handled.
Changes requested from review comment #22 (review) have been managed. The methods '_check_config_file_existence()', '_check_exe_file_existence()' and '_build_nVsKn()' have been changed into private methods.
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.
Modifications requested in the review of task 7 have been completely handled.
All the modifications required during the review process have been satisfactorily applied.
Before closing this pull request, I'd suggest to insert the UML diagram of the currently implemented code. |
All the required modifications have been applied successfully and the UML diagram of the resulting code architecture has been loaded. The branch can now be merged into the master branch. |
All the tasks concerning the issue #1 have been managed. Waiting for the review.