599 improved profiling modes - #601
Conversation
Matthew Hambley (MatthewHambley)
left a comment
Matthew Hambley (MatthewHambley)
left a comment
There was a problem hiding this comment.
A few small things.
| # as an always available dummy profile. | ||
| _inherit_from: dict[str, str] = {"": ""} | ||
|
|
||
| def __init__(self: "ProfileFlags", |
There was a problem hiding this comment.
We don't normally hint the self object reference, but if you want to you can from __future__ import annotations, then you don't need to quote the name.
There was a problem hiding this comment.
Oh, no idea what I was thinking there. Removed.
| :param inherit_from: Optional name of a profile to inherit | ||
| settings from. | ||
| ''' | ||
| if name in cls._inherit_from: |
There was a problem hiding this comment.
Is it safer to force name.lower()?
There was a problem hiding this comment.
Indeed.
| if name in cls._inherit_from: | ||
| raise KeyError(f"Profile '{name}' is already defined.") | ||
|
|
||
| if inherit_from is not None: |
There was a problem hiding this comment.
Another candidate for inherit_from.lower()?
There was a problem hiding this comment.
Yes, as well.
| ''' | ||
| if not file_path: | ||
| # If no path, provide a dummy path | ||
| file_path = Path() |
There was a problem hiding this comment.
This will be the currently selected directory. Is that the behaviour you want?
There was a problem hiding this comment.
It doesn't matter (atm only compiler and preprocessor actually use the profile flags, and they all provide a path), but it is indeed not necessary here. So, I've removed that.
|
Ready for next review. |
Joerg Henrichs (hiker)
added
the
Ready for review
2 participants
Closes #599.
It simplifies the definition of compilation profiles.
I added two commits for very minor cleanup of some tests (improved an error message, removed left-over debug print, and handled a warning, so pytest now reports all tests as green).
I have tested this with skeleton, lfric_atm, jules, um so far, and will verify that everything else is updated to use this new feature as well.