-
Notifications
You must be signed in to change notification settings - Fork 29
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
448 make public example #450
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #450 +/- ##
=========================================
Coverage ? 92.13%
=========================================
Files ? 85
Lines ? 13824
Branches ? 0
=========================================
Hits ? 12737
Misses ? 1087
Partials ? 0 ☔ View full report in Codecov by Sentry. |
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.
Looks good Joerg, thanks for adding to the docs and adding tests. Just a bit of tidying required.
doc/source/examples.rst
Outdated
|
||
create_dependencies.py | ||
^^^^^^^^^^^^^^^^^^^^^^ | ||
This file analysis the dependencies between a set of Fortran files, based on |
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.
s/analysis/analyses/
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.
Done.
doc/source/examples.rst
Outdated
that will read this file, execute the kernel, and compare the results with | ||
the original results. | ||
|
||
Since PSyclone will follow call tree, the code must be able to read even |
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.
"...follow the call tree,..."
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.
Done.
run time of the script, it will use these values as default values in the | ||
makefile. But by setting these environment variables when running ``make``, | ||
these defaults can always be overwritten. The Makefile also has a ``clean`` | ||
target, which will remove all ``.mod``, object, and the program file (if |
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.
Good stuff, thanks for adding this Joerg.
@@ -33,17 +33,22 @@ | |||
# ------------------------------------------------------------------------------ | |||
# Author: J. Henrichs, Bureau of Meteorology | |||
|
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.
Comment please, e.g. "Makefile driver to test the various examples"
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.
Done.
example/make_public.py
Outdated
# ----------------------------------------------------------------------------- | ||
# BSD 3-Clause License | ||
# | ||
# Copyright (c) 2020, Science and Technology Facilities Council. |
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.
Oh my life, this has been a while! Apologies. Could update to 2020-24 now?
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.
Oh no, not your fault. That's a copy&paste artefact (and I've fixed that)
example/make_public.py
Outdated
# ------------------------------------------------------------------------------ | ||
# Author: Joerg Henrichs, Bureau of Meteorology | ||
|
||
"""This file contains an fparser script that parses Fortran files |
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.
This docstring needs changing (and possibly explains the 2020 date ;-) ).
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.
Indeed :)
example/make_public.py
Outdated
all_nodes = list(walk(parse_tree, Attr_Spec)) | ||
for node in all_nodes: | ||
if str(node) == "PROTECTED": | ||
# This is a tuple, so we can't simple remove the attribute |
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.
"simply"
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.
Done.
for node in all_nodes: | ||
if str(node) == "PROTECTED": | ||
# This is a tuple, so we can't simple remove the attribute | ||
node.parent.items = tuple(i for i in node.parent.items if i is not node) |
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.
That's neat!
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.
Not sure tbh - given that modifying the tree is quite common task (I'd say), it's strange that we have tuples everywhere :(
I also did some very minor pylint fixes. Strangely, black allows more than 80 characters per line, it undid some of my fixes :) |
Adds a script that removes all private and protected attributes (which is used in PSyclone's kernel extraction).
Added a chapter explaining the examples to the manual, added tests for examples (triggered in CI), and added me as author.