search

mila-iqia / blocks-extras

A collection of extensions to the Blocks framework
MIT License
27 stars 40 forks source link

Pass only a subset of inputs to self.merge #51

Closed rizar closed 8 years ago

rizar commented 8 years ago

Sequence generators v2.0 were never properly finished, however it is essential to fix this bug because some code actually uses them. Methods scores and costs are supposed to receive more inputs than method merge can handle. Therefore, when they call merge, they should select only those necessary for merge by doing dict_subset(inputs, self.merge_names). In costs this has already been implemented, time to add it to scores as well.

Since in this repository tests and peer review are optional, I will merge this PR in the nearest future.

dmitriy-serdyuk commented 8 years ago

Did you interrupt the tests? I guessed that they are optional in a sense that you don't have to cover everything, but it is still nice to run them to make sure that the PR doesn't break the old code.

Otherwise, it looks fine, LGTM.

rizar commented 8 years ago

I did not interrupt them, not sure what happened.