(closes #2936, #1220, #2088) Fix doctests and add CI tests by sergisiso · Pull Request #3441 · stfc/PSyclone

sergisiso · 2026-05-26T10:55:13Z

No description provided.

…d brining the shallow example inside the test directory

codecov · 2026-06-01T11:57:10Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (a7ea7cc) to head (ce724c4).

Additional details and impacted files

@@            Coverage Diff            @@
##            master     #3441   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          393       393           
  Lines        55067     55046   -21     
=========================================
- Hits         55067     55046   -21

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…lities to be able to find them

sergisiso · 2026-06-09T13:20:43Z

@arporter @LonelyCat124 This PR fixes the doctests using both:
cd docs; make doctest
and
pytest -n auto --doctest-modules src/psyclone/

And enables both as checks in CI (We need both because there isn't a complete overlap between both sets and their working dir is different, so we need to be sure that paths have been normalised).

There are some doctests that didn't work, for example because they used non-existing example files. In this cases I converted the >>> to ... code-block :: which is not tested, and we can bringing them back to doctests over time.

It is ready for review.

arporter

Good job Sergi, it's great to have this working and the PR wasn't quite as big as I feared :-)
Only significant quibble is with the new utility method - I'm not sure it buys us much over get_invoke and wonder whether the two can be merged?

arporter · 2026-06-10T10:19:08Z


-.. testoutput::

+.. The snippet above fails a `.. testcode ::` block, but if it didn't we


Why does it fail? Is it because the file doesn't exist or is it because of the relative path given to the module manager?

It failed because some of the module files where moved to ../external/lfric_infrastructure/ and this was not updated. Fixed now.

(Hopefully these are the things that enabling doc-tests back will avoid)

arporter · 2026-06-10T10:23:55Z

+    config.api = api
+
+    _, info = parse(filepath, api=api)
+    psy = PSyFactory(api, distributed_memory=dist_mem).create(info)


This could be simplified a lot if you used get_invoke. In fact, I'm not sure that this new method brings much benefit over the latter?

I have now one calling the other, so the code is not duplicated.

One of the reasons that I prefered the new method is because all examples had the "invoke.schedule" before and this is not user-facing infrastructure. It's not what they have to do in the trans script and think this could be unecessarely confusing.

arporter · 2026-06-10T10:26:12Z


 class GOceanExtractTrans(ExtractTrans):
-    ''' GOcean1.0 API application of ExtractTrans transformation \
+    ''' GOcean1.0 API application of ExtractTrans transformation


Just "GOcean" now please.

arporter · 2026-06-11T10:38:25Z

    ''' LFRic API application of ExtractTrans transformation
    to extract code into a stand-alone program. For example:

-    >>> from psyclone.parse.algorithm import parse


How come you've removed this? Also, the paragraph above still ends with "For example:..."

The filename does not exist: find . -name solver_alg.x90 returns nothing.

Removed "For example:"

arporter · 2026-06-11T10:48:17Z

-    >>> # Uncomment the following line to see a text view of the schedule
-    >>> # print(schedule.view())
-    >>> kernels = schedule.children[9]
+    >>> kernels = schedule.children[26]


This could be a bit fragile. Could we make it a little better by walking over Loops?

Done.

Note that a lot of doctests like this one are currently not very interesting, they only show how to apply a transformation (which a python user should already know). What is more interesting is to show the before and after of the associated code. I suppose this was the idea behing the print(schedule.view()) but these are too lang and hard to read and they where commented in the documentation anyway (so not checked nor shown in docs).

My idea is to make a second chosing snippets of fortran that demonstrate each transformation, but it became too much for this PR. So for now I just wanted to enable the automatic testing and then we should make them apply and showcase more relevant inout/output nodes/strings.

arporter · 2026-06-11T11:08:27Z

 class OMPParallelLoopTrans(OMPLoopTrans):
    ''' Adds an OpenMP PARALLEL DO directive to a loop.

-        For example:


It seems a shame to lose this. Is it possible to keep a version?

As mention, I didn't find these very valuable as they were because:

the psyfactory/invoke should not be exposed to the user

the trans.apply is trivial and doesn't need a example imo

the input file does not exist, so it was not possible to see the behaviour (which is the interesting part).

But since you asked I redid this example as I think we should be doing them.

arporter · 2026-06-11T12:37:07Z


    '''
-    Profiler.set_options([Profiler.INVOKES], api="nemo")
+    Profiler.set_options([Profiler.INVOKES], api="gocean")


This implies Profiler was missing a check on the validity of the API string? Have you fixed that?

This didn't use the api value, and it feels it is not the right place to validate api strings. So I changed it to a boolean "is_psykal"

(btw, I initially changed that because I did a grep for remaining api="nemo", not because it failed)

arporter · 2026-06-11T12:40:40Z

-        assert issubclass(psy_except, PSycloneError)
+        # Check if the exception inherits PSycloneError (we don't use
+        # issubclass because the import_modules used in this test re-imports
+        # already loaded modules and create two super classes PSycloneError)


Does that mean we should be monkeypatching or something or are those changes local to this test?

(Well done on fixing this BTW - presumably this was the test error we see occasionally?)

Yes, this was not isolated. But under further inspection I think we were overcomplicating a lot this test. I updated to something much simpler. Let me know what you think.

arporter · 2026-06-11T12:46:23Z

-    >>> from psyclone.parse.algorithm import parse
-    >>> from psyclone.psyGen import PSyFactory
-    >>> api = "lfric"
-    >>> ast, invokeInfo = parse("file.f90", api=api)


Is it possible to recover this one? If not, please update the docstring above.

Updated docstring.

sergisiso · 2026-06-19T13:11:03Z

@arporter This is ready for another look, I provided in the comments some more explanation about why I did certain additions/deletetions in the PR that were missing for context.

sergisiso added 7 commits May 22, 2026 10:05

Fix some docstring tests

4c25522

Fix doctests in src/psyclone/psyir

53f4e7a

Fix remaining doctest (some by deleting or commenting them)

67027b6

Test docstring doctest in the CI pytest

3319d26

Fix some doctests by creating a get_psylayer_schedule test utility an…

43a9b5d

…d brining the shallow example inside the test directory

Fix several issues with the docs doctest check

d618ba6

Fix shpinx doctest and uncomment the CI test

8c6267d

Base automatically changed from scalartype_constructors to master May 26, 2026 14:11

sergisiso and others added 5 commits May 26, 2026 17:13

Merge branch 'master' into fix_doctests

4cc224e

Merge remote-tracking branch 'origin/master' into fix_doctests

f792571

Fix get_base_path test

8257754

Remove doctest FIXMEs

994a08d

Fix flake8 issue

38eba7b

sergisiso added 7 commits June 1, 2026 12:57

Use -e when testing if doctest passes

abcb476

Add tests to the package

ddfba4b

Add test to build-documentation action for make doctest to work properly

f8c9485

Try to rever pyproject change

ad88895

Merge remote-tracking branch 'origin/master' into fix_doctests

287c96c

Fix pydocs and improve test coverage

d7805b6

Remove duplicated test/examples files and instead update the test uti…

e6240ea

…lities to be able to find them

sergisiso requested review from LonelyCat124 and arporter June 9, 2026 13:13

sergisiso self-assigned this Jun 9, 2026

sergisiso added the ready for review label Jun 9, 2026

arporter added under review and removed ready for review labels Jun 10, 2026

arporter requested changes Jun 11, 2026

View reviewed changes

arporter removed the under review label Jun 11, 2026

arporter added the reviewed with actions label Jun 11, 2026

sergisiso added 6 commits June 12, 2026 16:05

Bring back ModuleManager testcode block

82b621e

Remove duplicated code in test utils and simplify Profile set_options

359e0d0

Add ompparallelloop doctest

46a296c

Merge remote-tracking branch 'origin/master' into fix_doctests

ea6160f

Add OpenMP doctest and simplify psyclone exceptions testing

10ab93b

Add missing coverage to exceptions test

ce724c4

sergisiso added ready for review and removed reviewed with actions labels Jun 19, 2026


		.. testoutput::

		.. The snippet above fails a `.. testcode ::` block, but if it didn't we

Conversation

sergisiso commented May 26, 2026

Uh oh!

codecov Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sergisiso commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arporter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergisiso commented Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Jun 1, 2026 •

edited

Loading

sergisiso commented Jun 9, 2026 •

edited

Loading