Assorted tutorial fixes against Cylc 8 #4869

wxtim · 2022-05-12T11:19:46Z

Correct get-wind-data

These changes close #4660

Requirements check-list

I have read CONTRIBUTING.md and added my name as a Code Contributor.
Contains logically grouped changes (else tidy your branch by rebase).
Does not contain off-topic changes (use other PRs for other changes).
Applied any dependency changes to both setup.cfg and conda-environment.yml.
Not currently tested - manually tested.
No change log entry required (tutorial workflow content).
No documentation update required.

Correct get-wind-data

MetRonnie · 2022-05-12T16:04:17Z

cylc/flow/etc/tutorial/cylc-forecasting-workflow/bin/consolidate-observations

-    x_coords = [datum[0] for datum in data]
-    y_coords = [datum[1] for datum in data]
+    x_coords = [datum[1] for datum in data]
+    y_coords = [datum[0] for datum in data]


Much better now

I wanted to add a map layer but couldn't find a nice way of doing it without adding another dependency. No thanks.

cylc/flow/etc/tutorial/cylc-forecasting-workflow/flow.cylc

… file

cylc/flow/etc/tutorial/runtime-tutorial/etc/python-job.settings

cylc/flow/etc/tutorial/consolidation-tutorial/flow.cylc

hjoliver · 2022-05-18T01:23:08Z

Failing test:

FAILED tests/unit/test_resources.py::test_get_resources_all[tutorial/runtime-tutorial]

cylc/flow/etc/tutorial/runtime-tutorial/etc/python-job.settings

cylc/flow/etc/tutorial/consolidation-tutorial/etc/python-job.settings

cylc/flow/etc/tutorial/runtime-introduction/etc/python-job.settings

MetRonnie · 2022-05-18T15:00:39Z

cylc/flow/etc/tutorial/runtime-introduction/etc/python-job.settings

@@ -0,0 +1,12 @@
+# WARNING: This file contains an anti-pattern


Any reason this one has not been made into a symlink?

It didn't pass the tests when it was a symlink.

Then it seems suspicious the others are passing

The problems:

The symlink points outside of the workflow source so gets broken on install

There is inconsistency in the test scripts between the three:

cylc-flow/cylc/flow/etc/tutorial/runtime-introduction/.validate

Lines 18 to 23 in 47ec638

set -eux

FLOW_NAME="$(< /dev/urandom tr -dc A-Za-z | head -c6)"

cylc install --workflow-name "$FLOW_NAME" --no-run-name

cylc validate --check-circular --icp=2000 "$FLOW_NAME"

cylc play --no-detach --abort-if-any-task-fails "$FLOW_NAME"

cylc clean "$FLOW_NAME"

cylc-flow/cylc/flow/etc/tutorial/consolidation-tutorial/.validate

Lines 19 to 21 in 9ad4c07

set -eux

cylc validate . --icp=2000

(runtime-tutorial does not even have a .validate script)

Yes. Might as well fix it in this PR, it's mission-creeped enow already - I haven't touched these files before, so I have no idea why they are failing - Although since I moved them into the repo it's probably my fault! 😄

There is inconsistency in the test scripts between the three:

Some of these workflows are committed in a broken state, the aim of the tutorial to be to get the student to fix them.

On examination the usage of runtime-introduction in .validate doesn't match the usage in Cylc-doc - I've made it match and now it should work.

I've opened a ticket to thoroughly check the docs #4892

Note we have been using symlinks with the tutorials for years when they were in Rose, using rsync to install. Since cylc install is rsync underneath I would expect it to be the same deal?

I'll have a look. I guess this is urgent - it ought to go in before the seminars

This seems to have sorted itself out. I don't know what was going on, but it no longer appears to be going.

Ah got it - cylc get-resources tutorial turns the symlinks into files.

But cylc install does not. Worth opening an issue about?

Answer: no. We might consider adding an option that would turn on this feature of rsync, but only if someone asks for it...

This reverts commit a42ecf5.

wxtim · 2022-06-27T10:30:06Z

@MetRonnie @oliver-sanders _ I think that this is both ready - I've double checked the discussions and I think that the PR's made since those discussions have sorted the problem.

I feel that this should go in ASAP now.

MetRonnie · 2022-06-27T16:06:24Z

cylc/flow/etc/tutorial/runtime-introduction/.validate

-cylc install --workflow-name "$FLOW_NAME" --no-run-name
+SRC=$(cylc get-resources tutorial 2>&1 | head -n1 | awk '{print $NF}')
+cylc install "${SRC}/runtime-introduction" --workflow-name "$FLOW_NAME" --no-run-name


Just a thought for consistency, instead of doing this here, how about making run-validate-tutorials do

+SRC=$(cylc get-resources tutorial 2>&1 | head -n1 | awk '{print $NF}') +cd "$SRC" -for FILE in $(echo ../../cylc/flow/etc/tutorial/*/.validate) ; do +for FILE in $(echo */.validate) ; do

MetRonnie · 2022-06-27T16:14:20Z

cylc/flow/etc/tutorial/runtime-introduction/etc/python-job.settings

@@ -0,0 +1,12 @@
+# WARNING: This file contains an anti-pattern


Ah got it - cylc get-resources tutorial turns the symlinks into files.

But cylc install does not. Worth opening an issue about?

Pass Path Variables to Job

8e4360c

Correct get-wind-data

wxtim requested a review from MetRonnie May 12, 2022 11:19

wxtim self-assigned this May 12, 2022

wxtim added the doc Documentation label May 12, 2022

wxtim added this to the cylc-8.0.0 milestone May 12, 2022

MetRonnie reviewed May 12, 2022

View reviewed changes

MetRonnie added tutorial-workflows small and removed doc Documentation labels May 12, 2022

wxtim marked this pull request as draft May 13, 2022 12:22

wxtim added 3 commits May 13, 2022 13:34

moved dodgy bit of Jinja we don't want people seeing into an included…

1948f1b

… file

Added python env setting to all tutorial workflows

0d74610

added a newline at the end of several files

9f19895

wxtim commented May 13, 2022

View reviewed changes

cylc/flow/etc/tutorial/runtime-tutorial/etc/python-job.settings Outdated Show resolved Hide resolved

wxtim marked this pull request as ready for review May 13, 2022 12:50

wxtim requested review from oliver-sanders and MetRonnie May 13, 2022 12:50

MetRonnie reviewed May 16, 2022

View reviewed changes

cylc/flow/etc/tutorial/consolidation-tutorial/flow.cylc Show resolved Hide resolved

replaced files with symlinks

ad67e28

wxtim requested review from hjoliver and MetRonnie May 17, 2022 14:35

wxtim commented May 18, 2022

View reviewed changes

cylc/flow/etc/tutorial/runtime-tutorial/etc/python-job.settings Outdated Show resolved Hide resolved

wxtim commented May 18, 2022

View reviewed changes

cylc/flow/etc/tutorial/consolidation-tutorial/etc/python-job.settings Outdated Show resolved Hide resolved

wxtim commented May 18, 2022

View reviewed changes

cylc/flow/etc/tutorial/runtime-introduction/etc/python-job.settings Outdated Show resolved Hide resolved

wxtim added 3 commits May 18, 2022 10:11

Correct symlinks

25e97c3

added shebang lines

d2f9b41

fixed broken symlink

a42ecf5

MetRonnie modified the milestones: cylc-8.0.0, cylc-8.0rc4 May 18, 2022

MetRonnie reviewed May 18, 2022

View reviewed changes

wxtim requested a review from MetRonnie May 19, 2022 07:16

wxtim added 2 commits May 20, 2022 09:46

Revert "fixed broken symlink"

5cb1832

This reverts commit a42ecf5.

modified validate script for runtime intro to match tutorial in cylc doc

1d8a5a6

wxtim removed the request for review from oliver-sanders May 20, 2022 09:08

wxtim requested a review from oliver-sanders June 17, 2022 09:30

wxtim changed the title ~~Pass Path Variables to Job~~ Assorted tutorial fixes against Cylc 8 Jun 21, 2022

MetRonnie reviewed Jun 27, 2022

View reviewed changes

MetRonnie approved these changes Jun 27, 2022

View reviewed changes

hjoliver approved these changes Jun 28, 2022

View reviewed changes

hjoliver merged commit d522ad8 into cylc:master Jun 28, 2022

wxtim deleted the tutorial.workflow_fixes branch June 28, 2022 09:11

hjoliver modified the milestones: cylc-8.0rc4, cylc-8.0.0 Jul 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Assorted tutorial fixes against Cylc 8 #4869

Assorted tutorial fixes against Cylc 8 #4869

wxtim commented May 12, 2022

MetRonnie May 12, 2022

wxtim May 13, 2022

hjoliver commented May 18, 2022

MetRonnie May 18, 2022

wxtim May 19, 2022

MetRonnie May 19, 2022

MetRonnie May 19, 2022

wxtim May 20, 2022 •

edited

Loading

oliver-sanders Jun 20, 2022

wxtim Jun 21, 2022

wxtim Jun 27, 2022

MetRonnie Jun 27, 2022 •

edited

Loading

MetRonnie Jun 28, 2022

wxtim commented Jun 27, 2022

MetRonnie Jun 27, 2022 •

edited

Loading

MetRonnie Jun 27, 2022 •

edited

Loading

		@@ -0,0 +1,12 @@
		# WARNING: This file contains an anti-pattern

	set -eux
	FLOW_NAME="$(< /dev/urandom tr -dc A-Za-z \| head -c6)"
	cylc install --workflow-name "$FLOW_NAME" --no-run-name
	cylc validate --check-circular --icp=2000 "$FLOW_NAME"
	cylc play --no-detach --abort-if-any-task-fails "$FLOW_NAME"
	cylc clean "$FLOW_NAME"

Assorted tutorial fixes against Cylc 8 #4869

Assorted tutorial fixes against Cylc 8 #4869

Conversation

wxtim commented May 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hjoliver commented May 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wxtim May 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MetRonnie Jun 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wxtim commented Jun 27, 2022

MetRonnie Jun 27, 2022 • edited Loading

Choose a reason for hiding this comment

MetRonnie Jun 27, 2022 • edited Loading

Choose a reason for hiding this comment

wxtim May 20, 2022 •

edited

Loading

MetRonnie Jun 27, 2022 •

edited

Loading

MetRonnie Jun 27, 2022 •

edited

Loading

MetRonnie Jun 27, 2022 •

edited

Loading