-
Notifications
You must be signed in to change notification settings - Fork 95
Ensure next/prev syntax works for --startcp option
#7024
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 8.6.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Ensure `next()/previous()` syntax works for the `cylc play --startcp` option. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,10 +123,11 @@ def _update_sources(self, other): | |
| ICP_OPTION = OptionSettings( | ||
| ["--initial-cycle-point", "--icp"], | ||
| help=( | ||
| "Set the initial cycle point." | ||
| " Required if not defined in flow.cylc." | ||
| "\nMay be either an absolute point or an offset: See" | ||
| f" {SHORTLINK_TO_ICP_DOCS} (Cylc documentation link)." | ||
| "Set the initial cycle point. " | ||
| "Required if not defined in flow.cylc.\n" | ||
| "Can be an absolute point or an offset relative to the " | ||
| "current time - see " | ||
| f"{SHORTLINK_TO_ICP_DOCS} (Cylc documentation link)." | ||
|
Comment on lines
+126
to
+130
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why change this?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For clarity |
||
| ), | ||
| metavar="CYCLE_POINT or OFFSET", | ||
| action='store', | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,6 @@ | |
| # along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
| """Common logic for "cylc play" CLI.""" | ||
|
|
||
| from ansimarkup import parse as cparse | ||
| import asyncio | ||
| from copy import deepcopy | ||
| from functools import lru_cache | ||
|
|
@@ -24,50 +23,62 @@ | |
| from pathlib import Path | ||
| from shlex import quote | ||
| import sys | ||
| from typing import TYPE_CHECKING, Tuple | ||
| from typing import ( | ||
| TYPE_CHECKING, | ||
| Tuple, | ||
| ) | ||
|
|
||
| from ansimarkup import parse as cparse | ||
| from packaging.version import Version | ||
|
|
||
| from cylc.flow import LOG, __version__ | ||
| from cylc.flow import ( | ||
| LOG, | ||
| __version__, | ||
| ) | ||
| from cylc.flow.exceptions import ( | ||
| CylcError, | ||
| ServiceFileError, | ||
| WorkflowStopped, | ||
| ) | ||
| from cylc.flow.scripts.ping import run as cylc_ping | ||
| import cylc.flow.flags | ||
| from cylc.flow.id import upgrade_legacy_ids | ||
| from cylc.flow.host_select import select_workflow_host | ||
| from cylc.flow.hostuserutil import is_remote_host | ||
| from cylc.flow.id import upgrade_legacy_ids | ||
| from cylc.flow.id_cli import parse_ids_async | ||
| from cylc.flow.loggingutil import ( | ||
| close_log, | ||
| RotatingLogFileHandler, | ||
| close_log, | ||
| ) | ||
| from cylc.flow.network.client import WorkflowRuntimeClient | ||
| from cylc.flow.network.log_stream_handler import ProtobufStreamHandler | ||
| from cylc.flow.option_parsers import ( | ||
| ICP_OPTION, | ||
| SHORTLINK_TO_ICP_DOCS, | ||
| WORKFLOW_ID_ARG_DOC, | ||
| CylcOptionParser as COP, | ||
| OptionSettings, | ||
| Options, | ||
| ICP_OPTION, | ||
| OptionSettings, | ||
| ) | ||
| from cylc.flow.pathutil import get_workflow_run_scheduler_log_path | ||
| from cylc.flow.remote import cylc_server_cmd | ||
| from cylc.flow.scheduler import Scheduler, SchedulerError | ||
| from cylc.flow.scripts.common import cylc_header | ||
| from cylc.flow.run_modes import WORKFLOW_RUN_MODES | ||
| from cylc.flow.workflow_db_mgr import WorkflowDatabaseManager | ||
| from cylc.flow.workflow_files import ( | ||
| SUITERC_DEPR_MSG, | ||
| get_workflow_srv_dir, | ||
| from cylc.flow.scheduler import ( | ||
| Scheduler, | ||
| SchedulerError, | ||
| ) | ||
| from cylc.flow.scripts.common import cylc_header | ||
| from cylc.flow.scripts.ping import run as cylc_ping | ||
| from cylc.flow.terminal import ( | ||
| cli_function, | ||
| is_terminal, | ||
| prompt, | ||
| ) | ||
| from cylc.flow.workflow_db_mgr import WorkflowDatabaseManager | ||
| from cylc.flow.workflow_files import ( | ||
| SUITERC_DEPR_MSG, | ||
| get_workflow_srv_dir, | ||
| ) | ||
|
|
||
|
|
||
| if TYPE_CHECKING: | ||
| from optparse import Values | ||
|
|
@@ -162,10 +173,14 @@ | |
| OptionSettings( | ||
| ["--start-cycle-point", "--startcp"], | ||
| help=( | ||
| "Set the start cycle point, which may be after" | ||
| " the initial cycle point. If the specified start point is" | ||
| " not in the sequence, the next on-sequence point will" | ||
| " be used. (Not to be confused with the initial cycle point)"), | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I notice leading spaces have crept into the codebase - not sure what the motivation is, as this is not how we write paragraphs ordinarily. Perhaps we should specify the style to follow to avoid inconsistency?)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ??? Whether the space goes at the start or end of the line is arbitrary, neither is "how we write paragraphs ordinarily" since we don't write this whitespace in plain text. So neither way is right or wrong, natural or unnatural, it's just a style choice like any other. Leading spaces are preferable because it is much easier to spot when a space has been omitted as they are all aligned in the first column (we have had several cases where tailing spaces were omitted) and with this arrangement commenting out or removing a line will never leave an erroneous space. It's exactly the same reason why placing operators at the start rather than end of the line is considered preferable (PEP8, flake8, black, ruff, prettier). FYI: There are no linting rules for strings, so style cannot be enforced.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @oliver-sanders |
||
| "Set the start cycle point, which may be after " | ||
| "the initial cycle point. " | ||
| "If the specified start point is not in the sequence, the next " | ||
| "on-sequence point will be used.\n" | ||
| "Can be an absolute point or an offset relative to the " | ||
| "current time, like the --initial-cycle-point option - see " | ||
| f"{SHORTLINK_TO_ICP_DOCS} (Cylc documentation link)." | ||
|
Comment on lines
+176
to
+182
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why change this? |
||
| ), | ||
| metavar="CYCLE_POINT", | ||
| action='store', | ||
| dest="startcp", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.