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

RSS and TZ #181

Closed
wants to merge 1 commit into
base: legacy/0.7
from

Conversation

Projects
None yet
2 participants
@vbeffara
Copy link

vbeffara commented Aug 18, 2013

Hi again,

I was having trouble generating an RSS feed, something to do with subtracting datestamps with and without timezone info: in feeds.py, epoch always has one but for dt it seems to dpend on what is specified as the 'date' field.

The attached patch attaches TZ info to epoch only if there is one in td.

What do you think of it?

@posativ

This comment has been minimized.

Copy link
Owner

posativ commented Aug 19, 2013

That's probably a bug in the post reader, because all datetime objects should be timezone aware. Do you use reST, MD or pandoc format? In case of YAML, with pyyaml installed?

The RSS feeds requires timezone aware dates because of daylight saving times the actual output string differs every half year.

@vbeffara

This comment has been minimized.

Copy link
Author

vbeffara commented Aug 19, 2013

That's using markdown, METASTYLE='native', with a timestamp format '%d.%m.%Y'.

Actually, something that might be worth changing: right now DATE_FORMAT is used both for creating new posts and for parsing previous ones. It would make sense to separate the two, and use something like python-dateutil for "agnostic" parsing instead of strptime (dateutil.parser.parse is extremely versatile, including options to set defaults for things like typically the timezone, for which using the local one when it is not specified is quite natural).

Reading the code, all my problems would probably be fixed by having TZINFO in my config file; a quickie would be to replace None with UTC in readers.py:261 ?

@vbeffara

This comment has been minimized.

Copy link
Author

vbeffara commented Aug 19, 2013

... BTW as I understand it, right now it is not possible to specify TZ in the date: field of a post? Using self.tzinfo feels more like a backup.

@posativ

This comment has been minimized.

Copy link
Owner

posativ commented Aug 19, 2013

It would make sense to separate the two, and use something like python-dateutil for "agnostic" parsing instead of strptime

python-dateutil is a dependency I try to avoid, because most features are not really needed here.

Your mentioned line readers.py:261 is actually quite senseless, because tzinfo is always set during the initialize step in commands.py:90. I still don't know where tz-unware datetime objects gets into the parsing/compilation flow.

@posativ

This comment has been minimized.

Copy link
Owner

posativ commented Aug 19, 2013

... BTW as I understand it, right now it is not possible to specify TZ in the date: field of a post? Using self.tzinfo feels more like a backup.

Yes, a tzinfo field would be nice if you're traveling around the world while writing. The current fallback assumes you're always in the same timezone.

@vbeffara

This comment has been minimized.

Copy link
Author

vbeffara commented Aug 19, 2013

OK, you're right about readers.py. But certainly the dt caught by the lambda in feeds.py:113 does not have a timezone info in my setup. And I'm still not familiar enough with all the callbacks and execution flow to trace where this comes from - as indeed IIRC it does have one out of readers.py ... I will investigate a bit more.

@posativ

This comment has been minimized.

Copy link
Owner

posativ commented Aug 19, 2013

Can you post a traceback by any chance?

@vbeffara

This comment has been minimized.

Copy link
Author

vbeffara commented Aug 19, 2013

Mmmmkay, found it. feeds.py:69 produces a tz-less value for updated on my system. This works: https://gist.github.com/anonymous/6268644

@posativ

This comment has been minimized.

Copy link
Owner

posativ commented Aug 19, 2013

Ah, right! That has been fixed in the master branch already... I think it is time to release 0.8 :S. Will you submit this patch or should I commit myself?

@vbeffara

This comment has been minimized.

Copy link
Author

vbeffara commented Aug 19, 2013

Just a sec, I will do a pull request.

@vbeffara vbeffara referenced this pull request Aug 19, 2013

Merged

Tz for updated #182

@posativ posativ closed this Aug 19, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.