Skip to content

Conversation

@james-bruten-mo
Copy link
Collaborator

Add a default for the groups run if the cylc variable can't be found.
The variable not being found is most likely because the suite was run without -z g= so used the suite's default group. This can change per repo, so label it with suite_default.

return starttime.split("+")[0]

def read_groups_run(self) -> str:
def read_groups_run(self) -> List[str]:
Copy link
Collaborator

@yaswant yaswant Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Various collection types from typing (List, Dict, etc) are now (Python 3.9+) considered legacy syntax in favour of built-in generic collection types (list, dict, etc).
https://docs.python.org/3/whatsnew/3.9.html#type-hinting-generics-in-standard-collections

We don't support anything for Python<3.9, do we? If we do, then I suggest we include from __future__ import annotations for backward compatibility, and use the build-in collection types.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There'll be quite a few to update, so let's change those in another PR

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to make an Issue for this so it doesn't get forgotten?

groups = ["suite_default"]
return groups

def get_task_states(self) -> Dict[str, str]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Comment on lines 76 to 79
self.task_states: Dict[str, str] = self.get_task_states()
self.groups: str = self.read_groups_run()
self.groups: List[str] = self.read_groups_run()
self.rose_data: Dict[str, str] = self.read_rose_conf()
self.dependencies: Dict[str, Dict] = self.read_dependencies()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

..and here

Copy link
Collaborator

@jennyhickson jennyhickson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Successfully parses the testing for MetOffice/lfric_core#170.

@jennyhickson jennyhickson merged commit 60f02d1 into MetOffice:main Dec 5, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants