adflow icon indicating copy to clipboard operation
adflow copied to clipboard

fprettify

Open sseraj opened this issue 3 years ago • 6 comments

Purpose

Formatting the Fortran code with fprettify

Expected time until merged

1 week

Type of change

  • [ ] Bugfix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (non-backwards-compatible fix or feature)
  • [x] Code style update (formatting, renaming)
  • [ ] Refactoring (no functional changes, no API changes)
  • [ ] Documentation update
  • [ ] Maintenance update
  • [ ] Other (please describe)

Testing

Current tests pass

Checklist

  • [x] I have run flake8 and black to make sure the code adheres to PEP-8 and is consistently formatted
  • [x] I have run unit and regression tests which pass locally with my changes
  • [ ] I have added new tests that prove my fix is effective or that my feature works
  • [ ] I have added necessary documentation

sseraj avatar Aug 18 '22 23:08 sseraj

fprettify is failing because of this issue https://github.com/pseewald/fprettify/issues/93 The issue has been fixed but is not available on the latest tag on pyPI.

sseraj avatar Aug 18 '22 23:08 sseraj

Codecov Report

Merging #229 (3ba85fe) into main (92fb234) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #229   +/-   ##
=======================================
  Coverage   40.77%   40.77%           
=======================================
  Files          13       13           
  Lines        3882     3882           
=======================================
  Hits         1583     1583           
  Misses       2299     2299           

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov[bot] avatar Aug 18 '22 23:08 codecov[bot]

fprettify is failing because of this issue pseewald/fprettify#93 The issue has been fixed but is not available on the latest tag on pyPI.

Do you wanna open an issue there to ask if they can make a new release?

ewu63 avatar Aug 19 '22 01:08 ewu63

I am converting this PR into a draft until we merge another PR that removes label statements. Labels are not considered best practice for F90 and cause problems with fprettify.

sseraj avatar Aug 22 '22 19:08 sseraj

@sseraj are the labels changed in #228? I just don't want this to get too stale since this potentially blocks all Fortran development.

ewu63 avatar Sep 20 '22 23:09 ewu63

The labels are not changed in #228. I will open another PR for the label fixes, but I have not had time to work on that lately. I will try to have the labels fixed by the time we merge #231, because we want to merge that PR before this one anyway.

sseraj avatar Sep 21 '22 14:09 sseraj

I don't know what's going on with the complex build because both of the last commits work for me locally. For context, I tried changing complexify.py to get it to stop replacing strings like "======" with ".ceq..ceq..ceq.". This was happening even before the fprettify changes but did not cause errors because the line character limit was not being exceeded. With the indenting changes in this PR, the line character limit is being exceeded.

sseraj avatar Feb 03 '23 21:02 sseraj

This is ready to be merged. @marcomangano @ArshSaja

The only Fortran file that has manual changes is src/preprocessing/preprocessingAPI.F90. The manual changes were necessary to fix the complex build issue that I mentioned above. I replaced the equal signs with dashes in the strings that were causing problems. Ideally, complexify.py would not change strings at all, but this would involve changes to the parser. I opened an issue for this in the complexify repo, which will eventually replace the current complex build process.

sseraj avatar Feb 06 '23 17:02 sseraj

Yes, I will open another PR updating .git-blame-ignore-revs after this is merged.

sseraj avatar Feb 20 '23 19:02 sseraj