2444 use dash instead of colon

[hiker]

A simple change that affects a lot of files. Fixes #2444.

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (afed0a9) 99.85% compared to head (4980605) 99.85%.

Files	Patch %	Lines
...clone/domain/lfric/lfric_extract_driver_creator.py	98.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2445      +/-   ##
==========================================
- Coverage   99.85%   99.85%   -0.01%     
==========================================
  Files         353      354       +1     
  Lines       47459    47856     +397     
==========================================
+ Hits        47390    47786     +396     
- Misses         69       70       +1     

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

[arporter]

This appears to have the CallTreeTools change in it too. Is it meant to?

[hiker]

Yes, I have 4 or so PRs lined up in a specific order. This issue eg. changes the driver names etc, so it's easier if it is done after #2296 and #1990 (and that's why I didn't label it as ready for review, but I added 'in progress' now to make this more obvious).

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.86%. Comparing base (05955d1) to head (7acac9b).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2445   +/-   ##
=======================================
  Coverage   99.86%   99.86%           
=======================================
  Files         352      352           
  Lines       48463    48463           
=======================================
  Hits        48398    48398           
  Misses         65       65           

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

[hiker]

I hope my greps worked all correct and I found all old : references. Note that this patch only affects the : in region names, NOT any colon added by e.g. psydata libraries when using the module- and region-name for creating region names (which are fine, since they are not creating files). I think it is consistent that we will leave it to the various psydata apps how to name things (and using : actually makes it possible to see what is region and what module ... before we had sometimes three :, meaning it was hard to see what was module and region (though de-facto it should be obvious). Now we have : between region and module, and - between the parts inside a region.

I did find a strange bug in examples/gocean/eg5:
The README states that the message ... module 'psy_test' region 'invoke_0:r0' is printed, but the message is actually module 'invoke_0' region 'r0'. I have no idea what happened (I doubt that would have make it through code review??). I tried to restore various old states of PSyclone, but had problems getting that example to run(??) - and decided it's nothing for this PR anyway. But it feels like a bug somewhere, the module name should indeed be psy_test, and region name invoke0-r0. I suggest we open a separate ticket for that?

[hiker]

I've also started the CI.

[arporter]

I did find a strange bug in examples/gocean/eg5:
The README states that the message ... module 'psy_test' region 'invoke_0:r0' is printed, but the message is actually module 'invoke_0' region 'r0'. I have no idea what happened (I doubt that would have make it through code review??). I tried to restore various old states of PSyclone, but had problems getting that example to run(??) - and decided it's nothing for this PR anyway. But it feels like a bug somewhere, the module name should indeed be psy_test, and region name invoke0-r0. I suggest we open a separate ticket for that?

Yes please.

[arporter]

Thanks for those updates Joerg.
Just a few small things to tidy up.
I see that the integration tests were all green.
Despite it being my suggestion to change them to colons (I think), I have to admit that I think the profiling region names looked better with colons but if you don't agree and/or it's a lot of work to change it back then I'm happy to leave this.

[arporter]

BTW, I'm loving the documentation with the code highlighting :-)

[hiker]

BTW, I'm loving the documentation with the code highlighting :-)

Me, too! The moment I saw this, I knew I had to introduce this to PSyclone :)

[hiker]

I hope I explained it a bit better where the : come from and that they are still there. I got confused when I picked this PR up as well, I had additional commits that removed all : in all profiling libraries, updated examples, tutorial, ... before I realised that I am doing stupid things :P

Given the costs of the CI, I won't trigger it again, since I only changed the user's guide.

[arporter]

All good now.
Thanks for clarifying.
Will proceed to merge once CI has run.