Fix #2653 - Make DATE_PATH follow rrule #2656

sukiletxe · 2017-01-31T18:45:47Z

No description provided.

sukiletxe · 2017-01-31T19:21:57Z

I don't know why Travis fails only in Python 3.5. Although Codacy fails, that part of the code was in the original. Should I fix it?

felixfontein · 2017-01-31T19:32:59Z

You can ignore the travis failure, it looks like a random fluke / test-related problem which has nothing to do with your changes.

Regarding the codacy issue: no, it wasn't there before your commit; before, it was calling datetime.datetime.now with that argument, and datetime.datetime.now is a callable function, while the result of datetime.datetime.strptime is not.

felixfontein · 2017-01-31T19:53:54Z

Your changes don't work for me:

$ nikola new_post -d
Creating New Post
-----------------

Title: test
Scanning posts......done!
Traceback (most recent call last):
  File "~/doit/doit/doit_cmd.py", line 167, in run
    return command.parse_execute(args)
  File "~/doit/doit/cmd_base.py", line 120, in parse_execute
    return self.execute(params, args)
  File "~/nikola-original/nikola/plugin_categories.py", line 130, in execute
    return self._execute(options, args)
  File "~/nikola-original/nikola/plugins/command/new_post.py", line 344, in _execute
    dateobj = datetime.datetime.strptime(data['date'].split('+')[0], '%Y-%m-%d %H:%M:%S')
  File "/usr/lib/python3.6/_strptime.py", line 565, in _strptime_datetime
    tt, fraction = _strptime(data_string, format)
  File "/usr/lib/python3.6/_strptime.py", line 365, in _strptime
    data_string[found.end():])
ValueError: unconverted data remains:  UTC

If I undo the change, I get:

$ nikola new_post -d
Creating New Post
-----------------

Title: test
Scanning posts......done!
[2017-01-31T19:53:24Z] INFO: new_post: Your post's text is at: posts/2017/01/31/test.rst

(as expected)

felixfontein · 2017-01-31T19:58:56Z

Printing data['date'] yields 2017-01-31 19:54:51 UTC, which explains why it doesn't work here.

You should modify get_date() (line 65 in the same file) to also return the date stamp itself, and then work with it instead of re-parsing the string returned by get_date().

sukiletxe · 2017-02-01T17:20:40Z

Done. Thanks @felixfontein for your comments.

felixfontein · 2017-02-01T21:34:39Z

The failing tests are completely unrelated to this PR. @Kwpolska: it looks like the baseline is out of date (a pygments update or so?). Also, there's a problem with Codacy coverage reporting:

$ if [[ $NMODE == 'nikola' ]]; then scripts/codacity_coverage.sh; fi
2017-02-01 17:08:30,876 - ERROR - environment variable CODACY_PROJECT_TOKEN is not defined.

felixfontein · 2017-02-01T21:39:14Z

How about replacing the tests in tests/test_scheduling.py by the following? Then the full return value of get_date() will be tested. (Disclaimer: I didn't test whether this actually works. I just did some replacements in a text editor.)

        # NOW does not match rule #########################################
        # No last date
        expected = TODAY.replace(day=23)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_FR, tz=UTC))
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_FR, tz=UTC))

        # Last date in the past; doesn't match rule
        date = TODAY.replace(hour=7)
        expected = TODAY.replace(day=23, hour=7)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_FR, date, tz=UTC))

        # Last date in the future; doesn't match rule
        date = TODAY.replace(day=24, hour=7)
        expected = TODAY.replace(day=30, hour=7)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_FR, date, tz=UTC))

        # Last date in the past; matches rule
        date = TODAY.replace(day=16, hour=8)
        expected = TODAY.replace(day=23, hour=8)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_FR, date, tz=UTC))

        # Last date in the future; matches rule
        date = TODAY.replace(day=23, hour=18)
        expected = TODAY.replace(day=30, hour=18)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_FR, date, tz=UTC))

        # NOW matches rule ################################################
        # Not scheduling should return NOW
        self.assertEqual((NOW, _NOW), get_date(False, RULE_TH, tz=UTC))
        # No last date
        self.assertEqual((NOW, _NOW), get_date(True, RULE_TH, tz=UTC))
        self.assertEqual((NOW, _NOW), get_date(True, RULE_TH, tz=UTC))

        # Last date in the past; doesn't match rule
        # Corresponding time has already passed, today
        date = TODAY.replace(day=21, hour=7)
        expected = TODAY.replace(day=29, hour=7)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_TH, date, tz=UTC))
        # Corresponding time has not passed today
        date = TODAY.replace(day=21, hour=18)
        expected = TODAY.replace(day=22, hour=18)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_TH, date, tz=UTC))

        # Last date in the future; doesn't match rule
        # Corresponding time has already passed, today
        date = TODAY.replace(day=24, hour=7)
        expected = TODAY.replace(day=29, hour=7)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_TH, date, tz=UTC))
        # Corresponding time has not passed today
        date = TODAY.replace(day=24, hour=18)
        expected = TODAY.replace(day=29, hour=18)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_TH, date, tz=UTC))

        # Last date in the past; matches rule
        # Corresponding time has already passed, today
        date = TODAY.replace(day=15, hour=7)
        expected = TODAY.replace(day=29, hour=7)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_TH, date, tz=UTC))
        # Corresponding time has already passed, today; rule specifies HOUR
        date = TODAY.replace(day=15, hour=7)
        expected = TODAY.replace(day=29, hour=9)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_TH + ';BYHOUR=9', date, tz=UTC))
        # Corresponding time has not passed today
        date = TODAY.replace(day=15, hour=18)
        expected = TODAY.replace(day=22, hour=18)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_TH, date, tz=UTC))

        # Last date in the future; matches rule
        # Corresponding time has already passed, today
        date = TODAY.replace(day=29, hour=7)
        expected = TODAY.replace(day=5, month=9, hour=7)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_TH, date, tz=UTC))
        # Corresponding time has not passed today
        date = TODAY.replace(day=22, hour=18)
        expected = TODAY.replace(day=29, hour=18)
        self.assertEqual((expected, expected.strftime(FMT)), get_date(True, RULE_TH, date, tz=UTC))

felixfontein · 2017-02-02T19:29:32Z

In the code I posted above, replace (expected, expected.strftime(FMT)) by (expected.strftime(FMT), expected); then it works.

The .replace('+00:00', ' UTC') is not necessary, and (NOW, _NOW) was correct.

felixfontein · 2017-02-02T19:30:25Z

Also, you can add yourself to AUTHORS.txt while you're at it.

Kwpolska · 2017-02-02T19:47:24Z

https://github.com/getnikola/nikola/blob/master/AUTHORS.txt#L105

felixfontein · 2017-02-02T19:48:29Z

Ah. Already there, then :)

sukiletxe · 2017-02-02T21:42:15Z

OK, I've screwed things up in a completely stupid way. Let me fix this. I'm really sorry for the trouble.

felixfontein · 2017-02-03T06:05:42Z

No problem! Take your time.

sukiletxe · 2017-02-03T11:12:48Z

OK, this is done at last. I'm sorry again for the trouble. By the way, there are two tests in test_scheduling.py (the first and the second), which I believe test the same thing (this is also in master).

sukiletxe · 2017-02-03T11:16:09Z

@felixfontein: Actually, if I understood the code correctly, the replacements are in fact necessary, as get_date returns UTC if the timezone is UTC, whereas %z returns +00:00.

felixfontein · 2017-02-03T19:55:05Z

But it isn't using %z, but %Z, which is supposed to return the name of the timezone (see https://docs.python.org/3/library/datetime.html#strftime-and-strptime-behavior; the examples explicitly mentions UTC). So the replacement should be unnecessary.

Which Python version under which operating system do you have which returns +00:00 instead of UTC for %Z?

sukiletxe · 2017-02-04T14:24:53Z

I think I saw Travis itself returning +00:00 in the failing tests. That's why I replaced it in the first place.

felixfontein · 2017-02-04T14:33:22Z

Well, so far it worked fine without the replace, both on travis and on my local machine (and on other machines as well). So it should be unnecessary.

@Kwpolska: what do you think? Are these replacements OK?

Kwpolska · 2017-02-04T17:47:00Z

These don’t match because two different methods are in use. new_post uses some special (more human) rules to produce those strings, while the test suite is using %Z. But do we really need to test those strings? Why not just test equality of datetime objects, now that get_date() also returns them?

sukiletxe · 2017-02-06T10:37:18Z

Now that I think about it, the replacements weren't necessary in the first tests update (I rebased too much and that diff is gone now, sorry).
Maybe testing equality could be a good idea (that is, equality between expected and get_date()[1]), as datetime.datetime.strftime is indeed predictable, although I don't see anything incorrect in the current behaviour. So as you wish, agree among yourselves and I'll do it (no rebases, I promise).

Kwpolska · 2017-02-06T17:16:38Z

Just change those tests to look only at the datetime object.

sukiletxe · 2017-02-06T19:14:15Z

Done.

Kwpolska · 2017-02-06T19:18:40Z

Travis tests fail because you are not a member of the @getnikola organization, and thus Travis prevents your PR from accessing the token (which is pretty smart from the security perspective). I fixed that on master for future PRs, and I’m merging yours.

Thank you for contributing to Nikola! 🎉

sukiletxe · 2017-02-06T19:25:39Z

Thank you for your patience (my coding skills are somewhat poor).

sukiletxe added 5 commits February 3, 2017 11:53

Fix #2653 - Make NEW_POST_DATE_PATH refollow rrule

054e70c

Update changelog for #2653

4bd8f6b

Fixed errors for #2653

cedad28

Updated tests for #2653

a7a4262

Fixed tests for #2653

6d6e6ed

Now tests for #2653 only check for datetime objects

7011848

Kwpolska approved these changes Feb 6, 2017

View reviewed changes

Kwpolska merged commit 95d869d into getnikola:master Feb 6, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #2653 - Make DATE_PATH follow rrule #2656

Fix #2653 - Make DATE_PATH follow rrule #2656

sukiletxe commented Jan 31, 2017

sukiletxe commented Jan 31, 2017

felixfontein commented Jan 31, 2017

felixfontein commented Jan 31, 2017

felixfontein commented Jan 31, 2017

sukiletxe commented Feb 1, 2017

felixfontein commented Feb 1, 2017

felixfontein commented Feb 1, 2017

felixfontein commented Feb 2, 2017

felixfontein commented Feb 2, 2017

Kwpolska commented Feb 2, 2017

felixfontein commented Feb 2, 2017

sukiletxe commented Feb 2, 2017

felixfontein commented Feb 3, 2017

sukiletxe commented Feb 3, 2017

sukiletxe commented Feb 3, 2017

felixfontein commented Feb 3, 2017

sukiletxe commented Feb 4, 2017 via email

felixfontein commented Feb 4, 2017

Kwpolska commented Feb 4, 2017

sukiletxe commented Feb 6, 2017

Kwpolska commented Feb 6, 2017

sukiletxe commented Feb 6, 2017 via email

Kwpolska commented Feb 6, 2017

sukiletxe commented Feb 6, 2017 via email

Fix #2653 - Make DATE_PATH follow rrule #2656

Fix #2653 - Make DATE_PATH follow rrule #2656

Conversation

sukiletxe commented Jan 31, 2017

sukiletxe commented Jan 31, 2017

felixfontein commented Jan 31, 2017

felixfontein commented Jan 31, 2017

felixfontein commented Jan 31, 2017

sukiletxe commented Feb 1, 2017

felixfontein commented Feb 1, 2017

felixfontein commented Feb 1, 2017

felixfontein commented Feb 2, 2017

felixfontein commented Feb 2, 2017

Kwpolska commented Feb 2, 2017

felixfontein commented Feb 2, 2017

sukiletxe commented Feb 2, 2017

felixfontein commented Feb 3, 2017

sukiletxe commented Feb 3, 2017

sukiletxe commented Feb 3, 2017

felixfontein commented Feb 3, 2017

sukiletxe commented Feb 4, 2017 via email

felixfontein commented Feb 4, 2017

Kwpolska commented Feb 4, 2017

sukiletxe commented Feb 6, 2017

Kwpolska commented Feb 6, 2017

sukiletxe commented Feb 6, 2017 via email

Kwpolska commented Feb 6, 2017

sukiletxe commented Feb 6, 2017 via email