Advanced exclusion syntax #2258

dvalters · 2017-04-28T09:23:49Z

Implements remaining features in issue #1960.

Exclusions can already be a simple list of points (as in PR #2244), but can now also be any valid datetime sequence.

3) Partial datetime (datetime recurrence) exclusion:

[[[ PT1H ! T12 ]]] Run hourly but not at 12:00 (from the ICP)

[[[ T-00 ! (T00, T06, T12, T18) ]]] Run hourly but not at 00:00, 06:00, 12:00, 18:00

[[[ PT5M ! T-15 ]]] Run 5 minutely but not at 15 minutes past the hour (from the ICP)

[[[ T00 ! W-1T00 ]]] Run daily at 00:00 except on Mondays

4) Sequences exclusion:

[[[ PT1H ! PT6H ]]] Run hourly (from the ICP) but not 6 hourly (from the ICP)

[[[ T-00 ! PT6H ]]] Run hourly on the hour but not 6 hourly on the hour

Same as [[[ T-00 ! T-00/PT6H ]]] (the T-00 context is implied for the excluded sequence)

Same as [[[ T-00 ! (T00, T06, T12, T18) ]]]

Same as [[[ PT1H ! (T00, T06, T12, T18) ]]] (ICP Dependent)

[[[ T12 ! T12/P15D ]]] Run daily at 12:00 except every 15th day

The exclusion(s) could be any valid recurrence (would we want to do this?)

e.g. [[[ R/^/P1H ! R5/20000101T00/P1D ]]]

Also adds an extra test suite advanced_exclusions to test-battery.

dvalters · 2017-04-28T09:50:56Z

~~I need to update the user guide to reflect these changes (edit: done)~~

duboviy · 2017-04-30T19:58:20Z

lib/cylc/time_parser.py

+                        if excl_off:
+                            exclusion_point += excl_off
+                        exclusion_points.append(exclusion_point)
+                    except:


To catch any exception use the except Exception, instead of except BaseException or just except.

Indicate the most specific exception type in except block.

For example:

# Bad try: mapping[key] except Exception: ... # Better try: mapping[key] except KeyError: ...

dvalters · 2017-05-08T15:16:20Z

~~(Some issues pointed out to fix first, so don't review/merge yet if you've not started.)~~

dvalters · 2017-05-12T10:24:02Z

This has been refactored now so that the individual exclusion sequences are not separate types - they are the same class as the ISO8601Sequence type, just treated as exclusions.

dvalters · 2017-05-12T11:29:39Z

lib/cylc/cycling/__init__.py

+        if ',' in exclusions:
+            if (not exclusions.strip().startswith('(') or not
+                    exclusions.strip().endswith(')')):
+                raise Exception("'%s': a list of exclusions must be "


@oliver-sanders you may have spaces before and after parentheses now.

oliver-sanders · 2017-05-12T14:29:53Z

I'm afraid I've found a bug.

[[[T-00 ! 2000]]]  # This works.
[[[T-00 ! (2000, PT2H)]]]  # This doesn't!

dvalters · 2017-05-12T15:43:43Z

OK, it will require a few changes if you want to combine points and sequences in the same exclusion list. Will have a look next week.

dvalters · 2017-05-12T16:52:35Z

You should be able to combine exclusion types in this format now: T-00 ! (2000, PT2H)

[[[T-00 ! 2000]]]  # This works.
[[[T-00 ! (2000, PT2H)]]]  # <-- This should work now.

hjoliver · 2017-05-13T11:42:57Z

This is looking really good. One question about integer cycling. By analogy with [[[PT1H ! PT6H]]] I presume [[[P1 ! P6]]] should work - but it doesn't?

dvalters · 2017-05-15T07:47:14Z

I will see if I can get this implemented today - requires some changes for integer sequence exclusions to be valid.

oliver-sanders · 2017-05-15T08:26:06Z

@hjoliver ~~The syntax is [[[R/P1!6]]] (#1928 added exclusions for integer cycling cug).~~ Sorry hadn't read your comment properly.

dvalters · 2017-05-15T14:03:41Z

Integer sequence exclusions now implemented.

oliver-sanders

This is working for me! I've a few comments but otherwise it looks good.

oliver-sanders · 2017-05-17T08:38:09Z

lib/cylc/cycling/iso8601.py

+
+                try:
+                    self.p_iso_exclusions.append(
+                        point_parse(str(exclusion_point)))


What is the purpose of this point_parse(str(point))?

yes, it is redundant - removed

oliver-sanders · 2017-05-17T08:40:46Z

lib/cylc/cycling/iso8601.py

@@ -281,9 +281,64 @@ def _iso_interval_nonzero(interval_string):
        return bool(interval)


+class ISO8601Exclusions(object):


The IntegerExclusions and ISO8601Exclusions classes look pretty much identical now. cylc.cycling.loader provides abstractions for points/sequences, would it be possible to use this to consolidate these two classes?

refactored as much as possible in latest commit

oliver-sanders · 2017-05-17T08:46:00Z

tests/cyclers/exclusions_advanced/suite.rc

+            graph = wibble
+
+        # Run hourly at 15 minutes past except every 3rd hour
+        [[[ T-15!T-15/PT3H ]]]


I would be good to test the implicit version here too (T-15!PT3H).

Added the implicit case too

oliver-sanders · 2017-05-17T08:48:26Z

tests/cyclers/exclusions_advanced/suite.rc

+        # Stacked sequences
+        [[[ T0230, PT3H!(T09,T12), T1945 ]]]
+            graph = dibble
+[runtime]


It would also be a good idea to add a test here with some arbitrary white-space around parenthesis to test for the parsing problem we had earlier.

Added some arbitrary whitespace to one of these suite tests. (This is tested in the unit tests already - under the integer ones - which all use the same parse method to strip the whitespace/parentheses.

oliver-sanders · 2017-05-17T08:50:48Z

tests/cyclers/integer_exclusions_advanced/suite.rc

+            graph = bar
+        [[[ P1 !(P2,6,8) ]]]
+            graph = qux
+[runtime]


Could we add a test for implicit start/end context for integer cycling as well e.g. R/1/P2!P3.

hjoliver · 2017-05-19T04:15:28Z

(this is generally looking good to me, just waiting for Oliver's points to be addressed before a final review)

dvalters · 2017-05-19T16:49:43Z

~~Will finish adding the remaining extra tests next week.~~

Added extra tests now.

oliver-sanders

Looks good to me!

hjoliver

Very nice.

duboviy

It would be better to use here custom type of Exception instead of general Exception itself.

duboviy · 2017-08-06T09:19:26Z

lib/cylc/cycling/__init__.py

+        if ',' in exclusions:
+            if (not exclusions.strip().startswith('(') or not
+                    exclusions.strip().endswith(')')):
+                raise Exception("'%s': a list of exclusions must be "


Same as mentioned above

duboviy · 2017-08-06T09:19:58Z

lib/cylc/cycling/__init__.py

+            expression3 = "T01/PT1H! PT8H, (T06, T09)"
+            expression4 = "T01/PT1H! T03, T06, T09"
+
+            self.assertRaises(Exception, parse_exclusion, expression1)


Same as mentioned above

hjoliver · 2017-08-06T20:46:35Z

@duboviy - fair point, in general. Would you like to raise a Pull Request, or are you just "observing from afar"?

duboviy · 2017-08-09T09:17:57Z

@hjoliver just observing from afar

matthewrmshin · 2017-08-09T12:04:13Z

Hi @duboviy Thanks for sending us your insight from afar 🔭 . Please rest assure that we are fixing these issues - as opportunities arise.

duboviy · 2017-08-09T12:17:06Z

@matthewrmshin great, thanks!

dvalters self-assigned this Apr 28, 2017

dvalters requested review from hjoliver and oliver-sanders April 28, 2017 14:32

matthewrmshin added this to the next release milestone Apr 28, 2017

duboviy suggested changes Apr 30, 2017

View reviewed changes

dvalters force-pushed the advanced-exclusion-syntax branch from 33339df to 16b49bf Compare May 11, 2017 15:48

dvalters commented May 12, 2017

View reviewed changes

oliver-sanders mentioned this pull request May 12, 2017

cylc graph: excluded tasks appearing #2280

Closed

hjoliver modified the milestones: soon, next release May 15, 2017

oliver-sanders reviewed May 17, 2017

View reviewed changes

dvalters added 6 commits May 19, 2017 17:22

Implement advanced exlcusion syntax and parsing

d30e500

Small fixes and extra unittests

eb70ce0

Added advanced_exclusions to test battery

7a1acb8

PEP8 compliance

248623d

Additions to the CUG and minor tidy

62a0284

Exclusion syntax refactoring

0057aa7

dvalters added 9 commits May 19, 2017 17:26

Modified test-battery to test further exclusion cases

b94fa83

Remove extraneous function, exceptions handling

7611820

Implied start point for exclusion and unit test

b58a58f

Tidying

a17120b

Fix bracket-space parsing issue

fb2bf9c

Re-jig to allow exclusions to be points AND sequences

7636dfd

Added separate IntegerExclusions class

3337374

Implements integer sequence exclusions

a763525

Updated CUG, added integer exclusions to test battery

5af2468

dvalters force-pushed the advanced-exclusion-syntax branch from 05157aa to 6a443b4 Compare May 19, 2017 16:31

Refactor Exclusion Class

c8dba9e

dvalters force-pushed the advanced-exclusion-syntax branch from 6a443b4 to c8dba9e Compare May 19, 2017 17:08

Added extra test-battery tests

1d925f3

oliver-sanders approved these changes May 22, 2017

View reviewed changes

hjoliver approved these changes May 23, 2017

View reviewed changes

hjoliver merged commit 1e078fe into cylc:master May 23, 2017

hjoliver mentioned this pull request Jun 8, 2017

Advanced exclusion graphing syntax #1960

Closed

duboviy reviewed Aug 6, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Advanced exclusion syntax #2258

Advanced exclusion syntax #2258

dvalters commented Apr 28, 2017 •

edited

Loading

dvalters commented Apr 28, 2017 •

edited

Loading

duboviy Apr 30, 2017

dvalters commented May 8, 2017 •

edited

Loading

dvalters commented May 12, 2017

dvalters May 12, 2017

oliver-sanders commented May 12, 2017

dvalters commented May 12, 2017

dvalters commented May 12, 2017 •

edited

Loading

hjoliver commented May 13, 2017

dvalters commented May 15, 2017

oliver-sanders commented May 15, 2017 •

edited

Loading

dvalters commented May 15, 2017

oliver-sanders left a comment

oliver-sanders May 17, 2017

dvalters May 19, 2017

oliver-sanders May 17, 2017

dvalters May 19, 2017

oliver-sanders May 17, 2017

dvalters May 22, 2017

oliver-sanders May 17, 2017

dvalters May 22, 2017

oliver-sanders May 17, 2017

dvalters May 22, 2017

hjoliver commented May 19, 2017

dvalters commented May 19, 2017 •

edited

Loading

oliver-sanders left a comment

hjoliver left a comment

duboviy left a comment

duboviy Aug 6, 2017 •

edited

Loading

duboviy Aug 6, 2017

hjoliver commented Aug 6, 2017

duboviy commented Aug 9, 2017

matthewrmshin commented Aug 9, 2017

duboviy commented Aug 9, 2017

		@@ -281,9 +281,64 @@ def _iso_interval_nonzero(interval_string):
		return bool(interval)


		class ISO8601Exclusions(object):

Advanced exclusion syntax #2258

Advanced exclusion syntax #2258

Conversation

dvalters commented Apr 28, 2017 • edited Loading

3) Partial datetime (datetime recurrence) exclusion:

4) Sequences exclusion:

dvalters commented Apr 28, 2017 • edited Loading

Choose a reason for hiding this comment

dvalters commented May 8, 2017 • edited Loading

dvalters commented May 12, 2017

Choose a reason for hiding this comment

oliver-sanders commented May 12, 2017

dvalters commented May 12, 2017

dvalters commented May 12, 2017 • edited Loading

hjoliver commented May 13, 2017

dvalters commented May 15, 2017

oliver-sanders commented May 15, 2017 • edited Loading

dvalters commented May 15, 2017

oliver-sanders left a comment

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hjoliver commented May 19, 2017

dvalters commented May 19, 2017 • edited Loading

oliver-sanders left a comment

Choose a reason for hiding this comment

hjoliver left a comment

Choose a reason for hiding this comment

duboviy left a comment

Choose a reason for hiding this comment

duboviy Aug 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hjoliver commented Aug 6, 2017

duboviy commented Aug 9, 2017

matthewrmshin commented Aug 9, 2017

duboviy commented Aug 9, 2017

dvalters commented Apr 28, 2017 •

edited

Loading

dvalters commented Apr 28, 2017 •

edited

Loading

dvalters commented May 8, 2017 •

edited

Loading

dvalters commented May 12, 2017 •

edited

Loading

oliver-sanders commented May 15, 2017 •

edited

Loading

dvalters commented May 19, 2017 •

edited

Loading

duboviy Aug 6, 2017 •

edited

Loading