2455 DynDofmaps split & rename

[oakleybrunt]

Closes #2455

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (699ceb2) 99.85% compared to head (73278ea) 99.85%.

❗ Current head 73278ea differs from pull request most recent head 79d80bf. Consider uploading reports for the commit 79d80bf to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2457   +/-   ##
=======================================
  Coverage   99.85%   99.85%           
=======================================
  Files         352      353    +1     
  Lines       47334    47342    +8     
=======================================
+ Hits        47264    47273    +9     
+ Misses         70       69    -1     

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

[arporter]

Hi @hiker, this could be one for you to review if Oakley is happy?

[oakleybrunt]

Hi @hiker, this could be one for you to review if Oakley is happy?

Fine with me :)

[hiker]

Very nice work (I am especially happy about how the number of lines and pylint warnings for dynamo_0p3 has decreased. I am aware that this is not just this PR but a lot of earlier work :), but still, really nice to see).
A few minor things. But I have one question, and might need @arporter 's opinion: typically we should have (at least one) test file that covers the corresponding source file. In this case, I would have expected a file .../tests/domain/lfric/lfric_dofmaps_test.py. As far as I can see, the tests are atm in .../tests/dynamo0p3_cma.py , the only test modified here ;) And indeed, this file does cover 100% of lfric_dofmaps.py, but I'd guess it covers actually more. So, @arporter , do we expect this to be done as part of the refactor?

[arporter]

So, @arporter , do we expect this to be done as part of the refactor?

Yes please.

[hiker]

So, @arporter , do we expect this to be done as part of the refactor?

Yes please.

It looks all good, except I still get lines not covered by the corresponding test:

psyclone/src/psyclone/tests$ pytest --cov-report term-missing --cov psyclone.domain.lfric.lfric_dofmaps  ./domain/lfric/lfric_dofmaps_test.py
...
domain/lfric/lfric_dofmaps_test.py ......                                                                                                                                    [100%]

---------- coverage: platform linux, python 3.10.12-final-0 ----------
Name                                                                   Stmts   Miss  Cover   Missing
----------------------------------------------------------------------------------------------------
/home/joerg/work/psyclone/src/psyclone/domain/lfric/lfric_dofmaps.py      94      4    96%   279, 281, 287-290

If I add test/dynamo0p3_cma.py to the list of tests to run, you have 100% coverage, so it might just require to maybe move one test from one file to the other (since dynamo0p3.py is not at all covered by tests/dynamo*py anyway :) ).

[hiker]

I've also triggered the integration tests, since everything else is fine!

[hiker]

Last outstanding issue addressed, CI and integration tests all successful. Proceeding to merge

[hiker]

All issues addressed. CI/Integration green, no need to rebuild docs. Proceeding to merge