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

Document GUI Standards. #7

Merged
merged 5 commits into from
Mar 5, 2024
Merged

Document GUI Standards. #7

merged 5 commits into from
Mar 5, 2024

Conversation

searscr
Copy link
Contributor

@searscr searscr commented Feb 28, 2024

Description of Changes:

Document GUI standards with some examples.

Related Items

To Test:

cd /path/to/my/local/garnet/repo/
git fetch origin pull/<PULL_REQUEST_NUMBER>/head:pr<PULL_REQUEST_NUMBER>
git switch pr<PULL_REQUEST_NUMBER>
conda activate <garnet_environment>
python -m pytest <item_to_test>

Check list for the pull request

  • I have read the [CONTRIBUTING]
  • I have read the [CODE_OF_CONDUCT]
  • I have added tests for my changes
  • I have updated the documentation accordingly

Check list for the reviewer

  • I have read the [CONTRIBUTING]
  • I have verified the proposed changes
  • best software practices
    • all internal functions have an underbar, as is python standard
    • clearly named variables (better to be verbose in variable names)
    • code comments explaining the intent of code blocks
  • All the tests are passing
  • The documentation is up to date
  • code comments added when explaining intent

@codecov-commenter
Copy link

codecov-commenter commented Feb 28, 2024

Codecov Report

Attention: Patch coverage is 93.75000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 88.82%. Comparing base (02245df) to head (0a709dc).

Files Patch % Lines
src/garnet/home/view.py 92.45% 4 Missing ⚠️
src/garnet/helpers/ui_elements/base_lineedit.py 95.45% 1 Missing ⚠️
src/garnet/helpers/ui_elements/base_statusbar.py 95.23% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next       #7      +/-   ##
==========================================
+ Coverage   84.09%   88.82%   +4.73%     
==========================================
  Files           7        9       +2     
  Lines          88      179      +91     
==========================================
+ Hits           74      159      +85     
- Misses         14       20       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


Buttons should also be given meaningful names, such as “Load Experiment Plan” rather than
generic names like “Load”. If multiple buttons exist in the View, it should be clear to the
use which input/output fields correspond to each button. This can be achieved through grouping
Copy link
Collaborator

Choose a reason for hiding this comment

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

user

@hetrickjm
Copy link

I recommend renaming the section called Algorithm Input Validation to something more like Data Input Validation. I like that there are two types of validation

  1. Field validation which focuses on making sure what the user entered into the field is correct for the filed type. and
  2. Validating the data itself.

It is not clear that all data entered by a user will be directly used in an algorithm. I would prefer a more generic term for this type of validation and not focus (limit) it to algorithms.

Error Handling, perhaps better would be visual error handling. This is not the error handling strategy, but more how to inform the user when something goes wrong. We should add a link in here to the Error Handling strategy (I think this is being covered by loggging).

@@ -0,0 +1,81 @@
"""BaseLineEdit
------------
Copy link
Collaborator

@mpatrou mpatrou Mar 5, 2024

Choose a reason for hiding this comment

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

please create a folder called utility and inside create another called UIelements or similar; include the base_lineedit.py and base_statusbar.py there.

Copy link
Collaborator

@mpatrou mpatrou left a comment

Choose a reason for hiding this comment

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

works as expected

@searscr searscr merged commit 12df72d into next Mar 5, 2024
4 checks passed
@searscr searscr deleted the ewm3843 branch March 5, 2024 15:18
ktactac-ornl pushed a commit that referenced this pull request Mar 5, 2024
* Document GUI Standards.

* update pics

* update view and docs

* move helper classes

* remove required style if default value
@searscr searscr restored the ewm3843 branch March 5, 2024 20:20
@searscr searscr deleted the ewm3843 branch March 5, 2024 22:20
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