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

Ability to pass reference date context #74

Open
niedakh opened this issue Jul 12, 2018 · 3 comments
Open

Ability to pass reference date context #74

niedakh opened this issue Jul 12, 2018 · 3 comments

Comments

@niedakh
Copy link

niedakh commented Jul 12, 2018

I would like to be able to pass reference date to the builtin entity parser, just like in duckling or rustling. At the moment next friday matches the closest friday after parsing time. I would like to be able to get the closest friday to any arbitrary date when parsing next friday.

This is supported in rustling via the ResolverContext and could be also supported in the snips-ontology.

Two things are needed for this to work:

Replace:

let context = ResolverContext::default();

with a self.context property constructed based on provided datetime passed down to the rustling parser:
.parse_with_kind_order(&sentence.to_lowercase(), &context, &kind_order)

Add an ISO reference datetime in constructor:


convert it to ISO date string property or anything else that can be parsed by chrono in rust and push it down the binding slide via:
pub fn create_builtin_entity_parser(
ptr: *mut *const CBuiltinEntityParser,
lang: *const libc::c_char,
) -> Result<()> {
let lang = unsafe { CStr::from_ptr(lang) }.to_str()?;
let lang = Language::from_str(&*lang.to_uppercase())?;
let parser = BuiltinEntityParser::new(lang);
let c_parser = CBuiltinEntityParser(parser.into_raw_pointer() as _).into_raw_pointer();

to:

Then parse the string with chrono and set it up with:

let context = ResolverContext::new(Interval::starting_at(Moment(Local.ymd(year, month, day).and_hms(hour, minute, second)), Grain::Second));

Is this somewhere on your todo list? Are you working on this? Would you consider implementing this or helping me get to a point where I can submit an acceptable PR?

@niedakh
Copy link
Author

niedakh commented Jul 15, 2018

hmm. 4 days passed, you know, a simple yes/no/gtfo would be nice :)

@adrienball
Copy link
Collaborator

Hey @niedakh,
I apologize for the late answer, I've been very busy during the past few days and your issue got somehow lost in my todo 😞
What you suggest is definitely worth having, and the steps you described are exactly what needs to be done. This is not in our todo at the moment so that would be awesome if you could find some time to contribute on this ! If you decide to move forward on this, I'll follow up closely and help you get your PR ready for merge.

Thanks!
Adrien

@niedakh
Copy link
Author

niedakh commented Jul 17, 2018

@adrienball I've submitted the PR: #78, please review it at your convenience :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants