Skip to content
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

docs(workflow): add how to run non-contiguous dates and case lists #211

Open
wants to merge 4 commits into
base: feat(workflow)--add-fsdproc-parameters
Choose a base branch
from

Conversation

hollandjg
Copy link
Collaborator

No description provided.

@hollandjg hollandjg marked this pull request as ready for review December 19, 2024 22:15
Copy link
Member

@cpaniaguam cpaniaguam left a comment

Choose a reason for hiding this comment

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

Looks good. One of the scripts could be improved with some annotations.

```bash
cylc tui
```
In the TUI you can view logs, and "trigger" (i.e., rerun) tasks which failed.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In the TUI you can view logs, and "trigger" (i.e., rerun) tasks which failed.
In the TUI you can view logs, and "trigger" (i.e., rerun) failed tasks.

Comment on lines +75 to +78
for fullname in $(pipx run example/util/get_fullnames.py "${datafile}" "${index_col}" --start 1 --stop 10);
do
cylc vip . -n ${name} --run-name=${fullname} $(pipx run example/util/template.py ${datafile} ${index_col} ${fullname});
done
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for fullname in $(pipx run example/util/get_fullnames.py "${datafile}" "${index_col}" --start 1 --stop 10);
do
cylc vip . -n ${name} --run-name=${fullname} $(pipx run example/util/template.py ${datafile} ${index_col} ${fullname});
done
cases=$(pipx run example/util/get_fullnames.py "${datafile}" "${index_col}" --start 1 --stop 10)
for fullname in ${cases}; do
cylc vip . -n ${name} --run-name=${fullname} $(pipx run example/util/template.py ${datafile} ${index_col} ${fullname});
done

cylc tui
```

The `template.py` script provided doesn't currently have support for setting any other parameters, but could be extended if needed.
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be nice to have a link to template.py as you've provided for the config files.

```

The `template.py` script provided doesn't currently have support for setting any other parameters, but could be extended if needed.

## Oscar

Contact the Wilhelmus Lab members for access to the pipeline environment on Oscar, Brown University's High Performance Computing cluster.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Contact the Wilhelmus Lab members for access to the pipeline environment on Oscar, Brown University's High Performance Computing cluster.
Contact the Wilhelmus Lab members for access to the pipeline environment on Brown University's High Performance Computing cluster (Oscar).

Comment on lines +16 to +28
def main(
datafile: pathlib.Path,
index_col: str,
start: Optional[int] = None,
stop: Optional[int] = None,
step: Optional[int] = None,
):
f = pandas.read_csv(datafile)
print("\n".join(f[index_col].values[start:stop:step]))


if __name__ == "__main__":
typer.run(main)
Copy link
Member

Choose a reason for hiding this comment

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

It feels to me like this cli would benefit from some annotations/documentation. Is this warranted? What's your vision?

Copy link
Member

Choose a reason for hiding this comment

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

Here is what the help looks like right now; looks a bit too bare.

$ python get_fullnames.py --help

 Usage: delete.py [OPTIONS] DATAFILE INDEX_COL

╭─ Arguments ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ *    datafile       PATH  [default: None] [required]                                                                                                                                                  │
│ *    index_col      TEXT  [default: None] [required]                                                                                                                                                  │
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
╭─ Options ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ --start        INTEGER  [default: None]                                                                                                                                                               │
│ --stop         INTEGER  [default: None]                                                                                                                                                               │
│ --step         INTEGER  [default: None]                                                                                                                                                               │
│ --help                  Show this message and exit.           

Copy link
Member

Choose a reason for hiding this comment

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

Consider annotating get_fullnames.py in a similar fashion as this one. I can easily infer what this one script is for thanks to its annotations.

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.

2 participants