feat/timezone_config

[JarbasAl]

• all datetime extractors now default to now_local for anchorDate
• all localized_functions now inject a tzinfo in all tz-naive datetime objects passed as arguments
• default tz-info can be set with lingua_franca.time.set_default_tz
• tz-info injection can be enabled/disable in lingua_franca.config(default: enabled)

in what relates to mycroft-core:

• core should set the default timezone when setting language at intent step
• core should expect LF to always return tz-aware objects (in user configured timezone from .conf)

[krisgesling]

I wanted to clarify the points in the PR description as that will likely change my inline comments below.

• all datetime extractors now default to now_local for anchorDate

This to me is the bug and what we're really trying to fix.

all localized_functions now inject a tzinfo in all tz-naive datetime objects passed as arguments

This seems to be a feature made possible by the default tz config however I'm unclear as to how you are suggesting it work. Is it 1, 2, both, or something else?

1. LF users can pass in TZ naive datetime objects and they will have default tz info added.
2. LF users can choose to have all datetime objects converted to the default tz.

A possible issue arises in nice_duration. If I pass in 1 tz aware and 1 tz naive datetime, do I expect them both to be treated with the tzinfo of the tz aware datetime, or with my default tz info? I can see people making strong arguments in both directions, which to me adds weight to requiring that all datetime objects passed as parameters must be tz aware.

Enforcing the use of tz aware datetime objects reduces the chance of perceived bugs by warning downstream developers if they are using tz naive datetime objects. It also reduces the amount of datetime modifications we are doing.

tz-info injection can be enabled/disable in lingua_franca.config(default: enabled)

For this I assume number 2 above, else why would it be disabled?

default tz-info can be set with lingua_franca.time.set_default_tz

Wondering if this should get moved to lingua_franca.config.set_default_tz

in what relates to mycroft-core:
core should set the default timezone when setting language at intent step
core should expect LF to always return tz-aware objects (in user configured timezone from .conf)

If the intention was only to fix the bug in extract_datetime then we could add a tz parameter to that method and have mycroft-core wrap it providing the backwards compatibility for all Skills effectively:
extract_datetime(text, anchorDate=None, default_tz=now_local(), default_time=None)

I don't think we'd want to have all datetimes returned in local tz by default. However it seems we're only talking about extract_datetime that actually returns a datetime anyway right? In which case I would expect it be returned in the timezone of anchorDate or the default_tz if no anchorDate is provided.

[JarbasAl]

this method had not been migrated from mycroft-core, added in order to deprecate the mycroft side function as described in #182 (comment)

[ChanceNCounter]

@chrisveilleux I do see value in implementing this directly in lingua_franca.config, but I'm hesitant to extend the current config.py. #185 is currently roadmapped for the next "major" version (we're using minor numbers cuz "alpha") and it implements a proper config API.

The current config only contains one value, and it's just a top-level variable. The file wouldn't exist at all, except Neon and Chatterbox needed config.load_langs_on_demand or they couldn't upgrade. There was no reason to deny them that in the 0.3.0 upgrade, but the consequence is now there's that ugly thing.

As I see it, there are two options here:

• Use a setter function, as seen above, which will become redundant as soon as Implement #128: runtime-editable config for default parameters and module settings #185 is done
• Add another top-level variable config.default_time_zone, which will cease to exist as soon as Implement #128: runtime-editable config for default parameters and module settings #185 is done

One way or another it's something that implementations will have to change again in the future, but that's why it's v0.x. The difference is just how disruptive the change will be. If we go with the setter function, we can deprecate that function, and take as long as we want to do so, leaving implementations plenty of time to catch up. If we go with config.py, implementations will have to change it just to make the LF0.4 --> LF0.5 upgrade.