search

stanfordnlp / cocoa

Framework for learning dialogue agents in a two-player game setting.
MIT License
158 stars 62 forks source link

make item representaiton consistent - still need fix #22

Closed hhexiy closed 7 years ago

hhexiy commented 7 years ago

Mostly changes in item representation in the selection event. We agreed to use dict item except for displaying, when the dict item is converted to a list (order item). I marked the place that needs to be fixed.

hhexiy commented 7 years ago

hmm.. event.data here is already a dict. To convert it to a list we need the kb object so that we know the order of attributes.

On Sat, Dec 24, 2016 at 12:40 PM anushabala notifications@github.com wrote:

@anushabala commented on this pull request.

In src/web/main/routes.py https://github.com/stanfordnlp/game-dialogue/pull/22:

@@ -137,6 +137,7 @@ def check_inbox():

         message = format_message("Your partner has joined the room.", True)

     elif event.action == 'leave':

         message = format_message("Your partner has left the room.", True)

  • TODO: not rendered correctly because event.data is a dict instead of a list

Oh hmmm.. Is it not possible to call KB.ordered_item_to_dict(event.data) here?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stanfordnlp/game-dialogue/pull/22, or mute the thread https://github.com/notifications/unsubscribe-auth/ABJYpobydTaDTYCt6t0cF1A16B759JBpks5rLYM6gaJpZM4LVR2X .

hhexiy commented 7 years ago

oh i see what you mean. yeah if we have access to the kb obj at display time, we can use dict obj everywhere and call get ordered item. maybe put it in the event?? On Sat, Dec 24, 2016 at 1:34 PM He He hhe.xiy@gmail.com wrote:

hmm.. event.data here is already a dict. To convert it to a list we need the kb object so that we know the order of attributes.

On Sat, Dec 24, 2016 at 12:40 PM anushabala notifications@github.com wrote:

@anushabala commented on this pull request.

In src/web/main/routes.py https://github.com/stanfordnlp/game-dialogue/pull/22:

@@ -137,6 +137,7 @@ def check_inbox():

         message = format_message("Your partner has joined the room.", True)

     elif event.action == 'leave':

         message = format_message("Your partner has left the room.", True)

  • TODO: not rendered correctly because event.data is a dict instead of a list

Oh hmmm.. Is it not possible to call KB.ordered_item_to_dict(event.data) here?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stanfordnlp/game-dialogue/pull/22, or mute the thread https://github.com/notifications/unsubscribe-auth/ABJYpobydTaDTYCt6t0cF1A16B759JBpks5rLYM6gaJpZM4LVR2X .

anushabala commented 7 years ago

Ahhh I see, we need access to the KB... hmm.. honestly the only reason we need a list here is to preserve the original ordering of attributes from the schema, so that the values are displayed in the same order. But I don't think that's a necessity since there are only 3-5 values at best. What do you think of just doing something simple here like creating a list from all the values in the dict, no matter what order they're in? (Ie [data[attr.name] for are in data.keys()] ?

On Sun, Dec 25, 2016 at 3:10 AM hhexiy notifications@github.com wrote:

oh i see what you mean. yeah if we have access to the kb obj at display

time, we can use dict obj everywhere and call get ordered item. maybe put

it in the event??

On Sat, Dec 24, 2016 at 1:34 PM He He hhe.xiy@gmail.com wrote:

hmm.. event.data here is already a dict. To convert it to a list we need

the kb object so that we know the order of attributes.

On Sat, Dec 24, 2016 at 12:40 PM anushabala notifications@github.com

wrote:

@anushabala commented on this pull request.

In src/web/main/routes.py

https://github.com/stanfordnlp/game-dialogue/pull/22:

@@ -137,6 +137,7 @@ def check_inbox():

message = format_message("Your partner has joined the room.", True)

elif event.action == 'leave':

message = format_message("Your partner has left the room.", True)

  • TODO: not rendered correctly because event.data is a dict instead of

    a list

Oh hmmm.. Is it not possible to call KB.ordered_item_to_dict(event.data)

here?

—

You are receiving this because you authored the thread.

Reply to this email directly, view it on GitHub

https://github.com/stanfordnlp/game-dialogue/pull/22, or mute the thread

< https://github.com/notifications/unsubscribe-auth/ABJYpobydTaDTYCt6t0cF1A16B759JBpks5rLYM6gaJpZM4LVR2X

.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/stanfordnlp/game-dialogue/pull/22#issuecomment-269101436, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLcv0ZCA1sKSGI2HizcAfPeQ1dINnJ6ks5rLZFYgaJpZM4LVR2X .

-- Anusha Balakrishnan M.S. Computer Science (Artificial Intelligence) Stanford University '17 anusha@cs.stanford.edu

hhexiy commented 7 years ago

Oh sorry I forgot to update: Since the attrs are sorted by get_ordered_attribute_subset we can sort them here in the same way :). Also agree that this is a minor issue. Will update the PR tomorrow.

On Sat, Dec 24, 2016 at 10:54 PM, anushabala notifications@github.com wrote:

Ahhh I see, we need access to the KB... hmm.. honestly the only reason we need a list here is to preserve the original ordering of attributes from the schema, so that the values are displayed in the same order. But I don't think that's a necessity since there are only 3-5 values at best. What do you think of just doing something simple here like creating a list from all the values in the dict, no matter what order they're in? (Ie [data[ attr.name] for are in data.keys()] ?

On Sun, Dec 25, 2016 at 3:10 AM hhexiy notifications@github.com wrote:

oh i see what you mean. yeah if we have access to the kb obj at display

time, we can use dict obj everywhere and call get ordered item. maybe put

it in the event??

On Sat, Dec 24, 2016 at 1:34 PM He He hhe.xiy@gmail.com wrote:

hmm.. event.data here is already a dict. To convert it to a list we need

the kb object so that we know the order of attributes.

On Sat, Dec 24, 2016 at 12:40 PM anushabala notifications@github.com

wrote:

@anushabala commented on this pull request.

In src/web/main/routes.py

https://github.com/stanfordnlp/game-dialogue/pull/22:

@@ -137,6 +137,7 @@ def check_inbox():

message = format_message("Your partner has joined the room.", True)

elif event.action == 'leave':

message = format_message("Your partner has left the room.", True)

  • TODO: not rendered correctly because event.data is a dict instead

    of a list

Oh hmmm.. Is it not possible to call KB.ordered_item_to_dict(event. data)

here?

—

You are receiving this because you authored the thread.

Reply to this email directly, view it on GitHub

https://github.com/stanfordnlp/game-dialogue/pull/22, or mute the thread

< https://github.com/notifications/unsubscribe-auth/ ABJYpobydTaDTYCt6t0cF1A16B759JBpks5rLYM6gaJpZM4LVR2X

.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/stanfordnlp/game-dialogue/ pull/22#issuecomment-269101436, or mute the thread https://github.com/notifications/unsubscribe-auth/ ABLcv0ZCA1sKSGI2HizcAfPeQ1dINnJ6ks5rLZFYgaJpZM4LVR2X .

-- Anusha Balakrishnan M.S. Computer Science (Artificial Intelligence) Stanford University '17 anusha@cs.stanford.edu

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stanfordnlp/game-dialogue/pull/22#issuecomment-269112366, or mute the thread https://github.com/notifications/unsubscribe-auth/ABJYph-_wxdUt4BKujE4SLSV4xfQDdHqks5rLhM-gaJpZM4LVR2X .