NREL / GEOPHIRES-X

MIT License
26 stars 21 forks source link

Update to allow HTML output for HIP-RA-X #150

Closed malcolm-dsider closed 3 months ago

malcolm-dsider commented 3 months ago

Use Rich text library to allow for optional HTML output of the results.

softwareengineerprogrammer commented 3 months ago

@malcolm-dsider check test is failing due pre-commit hooks not being applied (i.e. formatting and basic code checks).

Output from test log:

ruff.....................................................................Failed
- hook id: ruff
- exit code: 1
- files were modified by this hook

Fixed 9 errors:
- src/hip_ra_x/hip_ra_x.py:
    5 × Q000 (bad-quotes-inline-string)
    3 × F541 (f-string-missing-placeholders)
    1 × I001 (unsorted-imports)

Found 9 errors (9 fixed, 0 remaining).

black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted src/hip_ra_x/hip_ra_x.py

All done! ✨ 🍰 ✨
1 file reformatted, 34 files left unchanged.

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
debug statements (python)................................................Passed
pre-commit hook(s) made changes.
If you are seeing this message in CI, reproduce locally with: `pre-commit run --all-files`.
To run `pre-commit` as part of git workflow, use `pre-commit install`.
All changes made by hooks:
diff --git a/src/hip_ra_x/hip_ra_x.py b/src/hip_ra_x/hip_ra_x.py
index f9bd144..16b26b7 100644
--- a/src/hip_ra_x/hip_ra_x.py
+++ b/src/hip_ra_x/hip_ra_x.py
@@ -8,7 +8,6 @@
 from pathlib import Path

 import pint
-
 from rich.console import Console
 from rich.table import Table

@@ -944,13 +943,13 @@ def PrintOutputsHTML(self, inputs, outputs, output_filename: str = 'HIP.html'):
         self.logger.info(f'Init {__class__.__name__!s}: {__name__}')

         try:
-            inputs_table = Table(title="***SUMMARY OF INPUTS***")
+            inputs_table = Table(title='***SUMMARY OF INPUTS***')
             inputs_table.add_column('Parameter Name', no_wrap=True)
-            inputs_table.add_column('Value', no_wrap=True, justify="center")
+            inputs_table.add_column('Value', no_wrap=True, justify='center')
             inputs_table.add_column('Units', no_wrap=True)
-            outputs_table = Table(title="***SUMMARY OF RESULTS***")
+            outputs_table = Table(title='***SUMMARY OF RESULTS***')
             outputs_table.add_column('Result Name', no_wrap=True)
-            outputs_table.add_column('Value', no_wrap=True, justify="center")
+            outputs_table.add_column('Value', no_wrap=True, justify='center')
             outputs_table.add_column('Units', no_wrap=True)

             for key, value in inputs['SUMMARY OF INPUTS'].items():
@@ -983,10 +982,10 @@ def PrintOutputsHTML(self, inputs, outputs, output_filename: str = 'HIP.html'):
                         unit = unit.replace('deg', '\u00b0')
                 outputs_table.add_row(name, val, unit)

-            console = Console(style="bold white on blue", force_terminal=True, record=True)
-            console.print(f'                  *********************')
-            console.print(f'                  ***HIP CASE REPORT***')
-            console.print(f'                  *********************')
+            console = Console(style='bold white on blue', force_terminal=True, record=True)
+            console.print('                  *********************')
+            console.print('                  ***HIP CASE REPORT***')
+            console.print('                  *********************')
             console.print(' ')
             console.print(inputs_table)
             console.print(' ')
@@ -1020,7 +1019,6 @@ def PrintOutputsHTML(self, inputs, outputs, output_filename: str = 'HIP.html'):
             self.logger.critical(msg)
             raise

-
     def __str__(self):
         return 'HIP_RA_X'

check: exit 1 (20.09 seconds) /home/runner/work/GEOPHIRES-X/GEOPHIRES-X> pre-commit run --all-files --show-diff-on-failure pid=1919
  check: FAIL code 1 (31.45=setup[4.35]+cmd[0.38,6.64,20.09] seconds)
  evaluation failed :( (31.54 seconds)
Error: Process completed with exit code 1.

pre-commit setup is described in Step 3 of Local Setup instructions: https://github.com/NREL/GEOPHIRES-X/blob/main/CONTRIBUTING.rst#local-setup. To resolve this, set up pre-commit as described in Step 3, then:

  1. Run pre-commit run --all-files --show-diff-on-failure
  2. git add . or git add <file> for individual files if you have other in-flight work you don't want to include in the PR right now.
  3. git commit -m "Fix check errors (pre-commit)"
  4. pre-commit will either pass or fail. If it fails and modifies files, go back to step 2 and continue. If it fails without modifying files (giving a message along the lines that it can't fix or only unsafe fixes are available), you will have to look at the individual failures/error codes and discern the resolution. Feel free to post those on this thread and I can try to take a look.
malcolm-dsider commented 3 months ago

All good. Thanks for the instructions that got me over the hurdle.

Is there a way for ruff, black, etc. to run automatically when I commit, or do I have to do it manually every time? If I have to do it manually, is there a way to set a macro or something so I don’t have to look up the command every time?

-Malcolm

From: Jonathan Pezzino @.> Sent: Friday, March 8, 2024 9:49 AM To: NREL/GEOPHIRES-X @.> Cc: Malcolm Ross @.>; Mention @.> Subject: Re: [NREL/GEOPHIRES-X] Update to allow HTML output for HIP-RA-X (PR #150)

@malcolm-dsider https://github.com/malcolm-dsider check test is failing due pre-commit hooks not being applied (i.e. formatting and basic code checks).

Output from test log https://github.com/NREL/GEOPHIRES-X/actions/runs/8194614493/job/22411070465?pr=150 :

ruff.....................................................................Failed

Fixed 9 errors:

Found 9 errors (9 fixed, 0 remaining).

black....................................................................Failed

reformatted src/hip_ra_x/hip_ra_x.py

All done! ✨ 🍰 ✨ 1 file reformatted, 34 files left unchanged.

trim trailing whitespace.................................................Passed fix end of files.........................................................Passed debug statements (python)................................................Passed pre-commit hook(s) made changes. If you are seeing this message in CI, reproduce locally with: pre-commit run --all-files. To run pre-commit as part of git workflow, use pre-commit install. All changes made by hooks: diff --git a/src/hip_ra_x/hip_ra_x.py b/src/hip_ra_x/hip_ra_x.py index f9bd144..16b26b7 100644 --- a/src/hip_ra_x/hip_ra_x.py +++ b/src/hip_ra_x/hip_ra_x.py @@ -8,7 +8,6 @@ from pathlib import Path

import pint

from rich.console import Console from rich.table import Table

@@ -944,13 +943,13 @@ def PrintOutputsHTML(self, inputs, outputs, output_filename: str = 'HIP.html'): self.logger.info(f'Init {class.name!s}: {name}')

     try:

check: exit 1 (20.09 seconds) /home/runner/work/GEOPHIRES-X/GEOPHIRES-X> pre-commit run --all-files --show-diff-on-failure pid=1919 check: FAIL code 1 (31.45=setup[4.35]+cmd[0.38,6.64,20.09] seconds) evaluation failed :( (31.54 seconds) Error: Process completed with exit code 1.

pre-commit setup is described in Step 4 of Local Setup instructions: https://github.com/NREL/GEOPHIRES-X/blob/main/CONTRIBUTING.rst#local-setup. To resolve this, set up pre-commit as described in Step 4, then:

  1. Run pre-commit run --all-files --show-diff-on-failure
  2. git add . or git add for individual files if you have other in-flight work you don't want to include in the PR right now.
  3. git commit -m "Fix check errors (pre-commit)"
  4. pre-commit will either pass or fail. If it fails and modifies files, go back to step 2 and continue. If it fails without modifying files (giving a message along the lines that it can't fix or only unsafe fixes are available), you will have to look at the individual failures/error codes and discern the resolution. Feel free to post those on this thread and I can try to take a look.

— Reply to this email directly, view it on GitHub https://github.com/NREL/GEOPHIRES-X/pull/150#issuecomment-1985931349 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AWGVYT6F5KFLX3OY3LXNINLYXHMXPAVCNFSM6AAAAABELWKQ5CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBVHEZTCMZUHE . You are receiving this because you were mentioned. https://github.com/notifications/beacon/AWGVYTYD3QSUZMYD56XXGWLYXHMXPA5CNFSM6AAAAABELWKQ5CWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTWL3UFK.gif Message ID: @. @.> >

softwareengineerprogrammer commented 3 months ago

@malcolm-dsider yes, it should always run automatically, try

pre-commit install --install-hooks
pre-commit autoupdate

and see if it's triggered next commit

malcolm-dsider commented 3 months ago

I entered your command (below), then I tried a commit, after a small change to one file. I used the PyCharm built-in Git user interface to do the commit, and I couldn’t tell if the pre-commit stuff ran.

Next time, I will try the commit from the terminal prompt and see how that goes…

-Malcolm

From: Jonathan Pezzino @.> Sent: Friday, March 8, 2024 10:36 AM To: NREL/GEOPHIRES-X @.> Cc: Malcolm Ross @.>; Mention @.> Subject: Re: [NREL/GEOPHIRES-X] Update to allow HTML output for HIP-RA-X (PR #150)

@malcolm-dsider https://github.com/malcolm-dsider yes, it should always run automatically, try

pre-commit install --install-hooks pre-commit autoupdate

and see if it's triggered next commit

— Reply to this email directly, view it on GitHub https://github.com/NREL/GEOPHIRES-X/pull/150#issuecomment-1986019107 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AWGVYT5LHRHQEFI2RO2W7ITYXHSIBAVCNFSM6AAAAABELWKQ5CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBWGAYTSMJQG4 . You are receiving this because you were mentioned. https://github.com/notifications/beacon/AWGVYTYP7WLMRTPIUHBXXQ3YXHSIBA5CNFSM6AAAAABELWKQ5CWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTWMA7SG.gif Message ID: @. @.> >

malcolm-dsider commented 3 months ago

OK, so I tried again, this time from the terminal prompt, and I could see it run, giving me a “passed” on all pre-commit functions.

Many of the changes by ruff (or black?) were changes to how code was distributed across however many lines. Is there a way for it not to mess with how I do my line breaks and other readability improvements? I don’t think it does a very good job from a readability point of view. I made a specific effort to make the code readable, and this tool changed most of what I did. I understand that it may be trying to enforce PEP standards or whatnot, but if that comes at the expense of readability, I am against it.

From: @. @.> Sent: Friday, March 8, 2024 10:51 AM To: 'NREL/GEOPHIRES-X' @.>; 'NREL/GEOPHIRES-X' @.> Cc: 'Mention' @.***> Subject: RE: [NREL/GEOPHIRES-X] Update to allow HTML output for HIP-RA-X (PR #150)

I entered your command (below), then I tried a commit, after a small change to one file. I used the PyCharm built-in Git user interface to do the commit, and I couldn’t tell if the pre-commit stuff ran.

Next time, I will try the commit from the terminal prompt and see how that goes…

-Malcolm

From: Jonathan Pezzino @. @.> > Sent: Friday, March 8, 2024 10:36 AM To: NREL/GEOPHIRES-X @. @.> > Cc: Malcolm Ross @. @.> >; Mention @. @.> > Subject: Re: [NREL/GEOPHIRES-X] Update to allow HTML output for HIP-RA-X (PR #150)

@malcolm-dsider https://github.com/malcolm-dsider yes, it should always run automatically, try

pre-commit install --install-hooks pre-commit autoupdate

and see if it's triggered next commit

— Reply to this email directly, view it on GitHub https://github.com/NREL/GEOPHIRES-X/pull/150#issuecomment-1986019107 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AWGVYT5LHRHQEFI2RO2W7ITYXHSIBAVCNFSM6AAAAABELWKQ5CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBWGAYTSMJQG4 . You are receiving this because you were mentioned. https://github.com/notifications/beacon/AWGVYTYP7WLMRTPIUHBXXQ3YXHSIBA5CNFSM6AAAAABELWKQ5CWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTWMA7SG.gif Message ID: @. @.> >

softwareengineerprogrammer commented 3 months ago

@malcolm-dsider https://stackoverflow.com/a/58584557/21380804

malcolm-dsider commented 3 months ago

OK, but then there will be # fmt on and # fmt: off all over the code…

From: Jonathan Pezzino @.> Sent: Friday, March 8, 2024 11:11 AM To: NREL/GEOPHIRES-X @.> Cc: Malcolm Ross @.>; Mention @.> Subject: Re: [NREL/GEOPHIRES-X] Update to allow HTML output for HIP-RA-X (PR #150)

@malcolm-dsider https://github.com/malcolm-dsider https://stackoverflow.com/a/58584557/21380804

— Reply to this email directly, view it on GitHub https://github.com/NREL/GEOPHIRES-X/pull/150#issuecomment-1986084790 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AWGVYT7B7FBLANSWZ3CDIP3YXHWJDAVCNFSM6AAAAABELWKQ5CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBWGA4DINZZGA . You are receiving this because you were mentioned. https://github.com/notifications/beacon/AWGVYT6J6CJEGKGLW3SPUK3YXHWJDA5CNFSM6AAAAABELWKQ5CWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTWME73M.gif Message ID: @. @.> >

softwareengineerprogrammer commented 3 months ago

@malcolm-dsider you can disable on a per-file basis following this pattern https://github.com/NREL/GEOPHIRES-X/blob/2346d1cd23d200fd04b1ab051ba7b5d471c086c4/pyproject.toml#L10

softwareengineerprogrammer commented 3 months ago

@malcolm-dsider you'll need to add rich to setup.py to get tests to pass - see https://github.com/NREL/GEOPHIRES-X/blob/2de4377d62cd32a77a6bea45996edab3d6d1cce0/setup.py#L79

softwareengineerprogrammer commented 3 months ago

@malcolm-dsider approved, assigned to you to merge 👍