Skip to content

New packages: py-ford and dependencies#31107

Merged
adamjstewart merged 19 commits intospack:developfrom
lizzyd710:add-ford
Jun 16, 2022
Merged

New packages: py-ford and dependencies#31107
adamjstewart merged 19 commits intospack:developfrom
lizzyd710:add-ford

Conversation

@lizzyd710
Copy link
Copy Markdown
Contributor

This PR adds the Ford documentation framework along with necessary dependencies that didn't already have spack packages:

Leaving this as a draft PR until I get the maintainers squared away for the packages. In the meantime, for the tagged creators of these libraries/packages, please indicate if you are willing to be tagged as a maintainer for these spack packages.

@lizzyd710
Copy link
Copy Markdown
Contributor Author

Are there certain workflows missing from the repo that the checks are trying to find? The logs mention some files that are supposed to be in github-actions-v0.2 (linking to the directory at the last commit at the time of writing this) but in my fork and the upstream, it doesn't exist.

@lizzyd710 lizzyd710 marked this pull request as ready for review June 13, 2022 15:17
@tldahlgren
Copy link
Copy Markdown
Contributor

FWIW. The failing style checks (aside from some odd sort issues) are:

==> Running flake8 checks
[207](https://github.com/spack/spack/runs/6868076623?check_suite_focus=true#step:6:208)
==> Error: spack style found errors
[208](https://github.com/spack/spack/runs/6868076623?check_suite_focus=true#step:6:209)
var/spack/repos/builtin/packages/py-ford/package.py:12: [E501] line too long (117 > 88 characters)
[209](https://github.com/spack/spack/runs/6868076623?check_suite_focus=true#step:6:210)
var/spack/repos/builtin/packages/py-markdown-include/package.py:12: [E501] line too long (162 > 88 characters)
[210](https://github.com/spack/spack/runs/6868076623?check_suite_focus=true#step:6:211)
var/spack/repos/builtin/packages/py-markdown-include/package.py:21: [W293] blank line contains whitespace
[211](https://github.com/spack/spack/runs/6868076623?check_suite_focus=true#step:6:212)
var/spack/repos/builtin/packages/py-md-environ/package.py:12: [E501] line too long (108 > 88 characters)

@lizzyd710
Copy link
Copy Markdown
Contributor Author

Is the import statement being flagged in the style check just a result of some weirdness happening behind the scenes, or do I actually have to change it? I just used what was given when I ran spack create

@tldahlgren
Copy link
Copy Markdown
Contributor

Is the import statement being flagged in the style check just a result of some weirdness happening behind the scenes, or do I actually have to change it? I just used what was given when I ran spack create

Unfortunately, it has to be changed to pass the check. That command provides a template with things that need to be filled in or removed. If it's off then that needs to be adjusted. (I'll take a look.)

@lizzyd710
Copy link
Copy Markdown
Contributor Author

From the output of the GitHub action I can't really tell what change I need to make--do you have an example of what a correct import placement would look like? I thought it looked fine since I checked other packages and it looked similar, but maybe I missed something.

@tldahlgren
Copy link
Copy Markdown
Contributor

From the output of the GitHub action I can't really tell what change I need to make--do you have an example of what a correct import placement would look like? I thought it looked fine since I checked other packages and it looked similar, but maybe I missed something.

isort doesn't like the comment line containing the dash to appear before the import. The most consistent fix is to simply remove it. Most packages also retain only a single blank line between the License Identifier comment and the first import.

@lizzyd710
Copy link
Copy Markdown
Contributor Author

I can't tell why it's failing these tests. The only thing that I can understand is the line that says it can't find the package py-markdown-include, despite it being added in this PR. Any insight or help as to what is going wrong would be appreciated, because when I run spack unit-test locally I get a slew of different errors that don't show up here.

@tldahlgren
Copy link
Copy Markdown
Contributor

The python 2.7 checks remind me of some failures yesterday that were fixed. Can you rebase to develop? (One option is to click the Update branch here but then you'll have to git pull if you make any more local changes.)

@tldahlgren
Copy link
Copy Markdown
Contributor

The reason you're seeing your new packages in tests is that they are automatically pulled into some existing unit tests.

@tldahlgren
Copy link
Copy Markdown
Contributor

@adamjstewart @alalazo Do either of you know why this PR is having problems with the unit tests (esp. 2.7)?

@adamjstewart
Copy link
Copy Markdown
Member

No idea, but I'm seeing the same issue in #30916 so I would also love to know why.

@adamjstewart adamjstewart self-assigned this Jun 14, 2022
@adamjstewart
Copy link
Copy Markdown
Member

P.S. Remind me to review the Python packages before this is merged.

@tldahlgren tldahlgren requested a review from adamjstewart June 14, 2022 19:19
@tldahlgren
Copy link
Copy Markdown
Contributor

P.S. Remind me to review the Python packages before this is merged.

Just added you as a reviewer. That should help. 😉

* Created package

* Finished creating package
* Created package and started to add info

* Removed unneeded global/install options
@sethrj sethrj changed the title Add ford New packages: py-ford and dependencies Jun 15, 2022
@lizzyd710 lizzyd710 requested a review from tldahlgren June 15, 2022 19:58
@lizzyd710
Copy link
Copy Markdown
Contributor Author

No idea, but I'm seeing the same issue in #30916 so I would also love to know why.

For some reason, the py-markdown-include package.py was in a UTF-8 charset when all my other package files were in ASCII. When I fixed it (see latest commit) then everything worked fine.

tldahlgren
tldahlgren previously approved these changes Jun 15, 2022
Copy link
Copy Markdown
Contributor

@tldahlgren tldahlgren left a comment

Choose a reason for hiding this comment

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

LGTM. Deferring merge decision until @adamjstewart has a chance to review and, preferably, at least @wscullin.

Copy link
Copy Markdown
Contributor

@tldahlgren tldahlgren left a comment

Choose a reason for hiding this comment

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

Re-confirmed sha256. Deferring merge until @adamjstewart has a chance to revisit.

@adamjstewart adamjstewart merged commit 8c3b82c into spack:develop Jun 16, 2022
@lizzyd710 lizzyd710 deleted the add-ford branch June 16, 2022 19:15
bhatiaharsh pushed a commit to bhatiaharsh/spack that referenced this pull request Aug 8, 2022
* Created package and added description

* Add py-markdown-include

* Created package

* Finished creating package

* Added py-md-environ

* Added build dependencies

* Added other deps

* Add python-markdown-math (#4)

* Created package and started to add info

* Removed unneeded global/install options

* Figured out version spec for markdown-math

* Removed type=build from unnecessary dependencies

* Removed unneeded install/global options, added version spec to dependency

* Added wscullin as interim maintainer for packages

* Fixed style issues

* Took care of trailing whitespace

* Removed comment line before imports

* Changed file charset to match other packages

* Update var/spack/repos/builtin/packages/py-ford/package.py

Co-authored-by: Adam J. Stewart <[email protected]>

* Update var/spack/repos/builtin/packages/py-ford/package.py

Co-authored-by: Adam J. Stewart <[email protected]>

* Removed test dependency after review feedback

* Added new 6.1.12 version to py-ford

Co-authored-by: Seth R. Johnson <[email protected]>
Co-authored-by: Adam J. Stewart <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants