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

Provide timezone to Duckling API #1351

Open
nioc opened this issue Aug 17, 2023 · 4 comments
Open

Provide timezone to Duckling API #1351

nioc opened this issue Aug 17, 2023 · 4 comments

Comments

@nioc
Copy link

nioc commented Aug 17, 2023

Is your feature request related to a problem? Please describe.
I would like to use Duckling for handling date and duration but I can not provide the timezone, so the returned value is in the default (UTC-7) and

Describe the solution you'd like
Duckling parse API can handle timezone in a tz parameter, it should be useful that the BuiltinDuckling sent it (using the system one or a NER config attribute).

Describe alternatives you've considered
None.

Additional context
I'm using the Duckling Rasa docker which is not using the TZ environment variable.

Same issue on Rasa which was handled

This example show my request:

{
    "text": "shutodwn at 23:15",
    "tz": "Europe/Paris",
    "locale": "fr_FR"
}

Without the tz parameter Duckling answer 2023-08-16T23:15:00.000-07:00:

[
  {
    "body": "23:15",
    "start": 12,
    "value": {
      "values": [
        {
          "value": "2023-08-16T23:15:00.000-07:00",
          "grain": "minute",
          "type": "value"
        },
        {
          "value": "2023-08-17T23:15:00.000-07:00",
          "grain": "minute",
          "type": "value"
        },
        {
          "value": "2023-08-18T23:15:00.000-07:00",
          "grain": "minute",
          "type": "value"
        }
      ],
      "value": "2023-08-16T23:15:00.000-07:00",
      "grain": "minute",
      "type": "value"
    },
    "end": 17,
    "dim": "time",
    "latent": false
  }
]

With tz parameter, answer is correct (2023-08-17T23:15:00.000+02:00):

[
  {
    "body": "23:15",
    "start": 12,
    "value": {
      "values": [
        {
          "value": "2023-08-17T23:15:00.000+02:00",
          "grain": "minute",
          "type": "value"
        },
        {
          "value": "2023-08-18T23:15:00.000+02:00",
          "grain": "minute",
          "type": "value"
        },
        {
          "value": "2023-08-19T23:15:00.000+02:00",
          "grain": "minute",
          "type": "value"
        }
      ],
      "value": "2023-08-17T23:15:00.000+02:00",
      "grain": "minute",
      "type": "value"
    },
    "end": 17,
    "dim": "time",
    "latent": false
  }
]

A rapid fix for this should be in the builtin-duckling request function:

      const postData = querystring.stringify({
        text: utterance,
        locale: BuiltinDuckling.getCulture(language),
        tz: Intl.DateTimeFormat().resolvedOptions().timeZone,
      });
@nioc
Copy link
Author

nioc commented Aug 17, 2023

As a workaround, I created a custom-duckling.js which extends BuiltinDuckling:

const { BuiltinDuckling } = require('@nlpjs/builtin-duckling')
const querystring = require('querystring')

class CustomDuckling extends BuiltinDuckling {
  constructor (settings = {}) {
    super(settings)
  }

  request (utterance, language) {
    return new Promise((resolve, reject) => {
      const postData = querystring.stringify({
        text: utterance,
        locale: BuiltinDuckling.getCulture(language),
        tz: Intl.DateTimeFormat().resolvedOptions().timeZone,
        lang: language.toUpperCase(),
      })

      const options = {
        host: this.url.hostname,
        port: this.port,
        method: 'POST',
        path: this.url.pathname,
        headers: {
          'Content-Type': 'application/x-www-form-urlencoded',
          'Content-Length': postData.length,
        },
      }
      const req = this.client.request(options, (res) => {
        let result = ''
        res.on('data', (chunk) => {
          result += chunk
        })
        res.on('end', () => {
          try {
            const obj = JSON.parse(result)
            resolve(obj)
          } catch (err) {
            reject(err)
          }
        })
        res.on('error', (err) => reject(err))
      })
      req.on('error', (err) => reject(err))
      req.write(postData)
      req.end()
    })
  }
}

module.exports = CustomDuckling

Called at application start:

const CustomDuckling = require('./custom-duckling')

manager = new NlpManager({...})
manager.container.register('extract-builtin-??', new CustomDuckling({
  ducklingUrl: 'http://localhost:8000/parse',
}), true)

There is a downside in case the BuiltinDuckling request method changes...

@MarketingPip
Copy link

@nioc - not an official maintainer / nor contributor. Just frequent user of this library, hate to see it die.

That said - no way you could possibly dig into the request method to see if your required changes could be implemented without any breaking changes? If so - I suggest obviously making a PR for this. As I am sure a lot of people are wanting this.

@nioc
Copy link
Author

nioc commented Oct 17, 2023

Hello @MarketingPip I think a timezone parameter should be added in settings object provided to constructor.
I can do a PR, but before I'm waiting a validation from any owner/maintainer.

@MarketingPip
Copy link

@nioc completely agree. That said - let's see if I can make a bump on your behalf so we can get approval on this.

@jseijas - can you give a confirm on this? (Don't mean to jack the issues - but trying to help solve / close some!)

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