search

Lucky-Dhakad / semanticvectors

Automatically exported from code.google.com/p/semanticvectors
Other
0 stars 1 forks source link

Avoid using the Flag datastructure in API classes: TermTermVectorsFromLucene #29

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
TermTermVectorsFromLucene uses Flags.dimension which makes it confusing to use 
the class directly in a client. It would be better if the dimension was also a 
parameter to the constructor. 
I am not sure how important backwards compatibility is, but we could add an 
additional constructor that takes the dimension as an additional parameter and 
then change things so that a field is set either from the parameter or the new 
constructor or from Flags.dimension in the old constructor.

I can change things accordingly if this is accepted.

Original issue reported on code.google.com by johann.petrak@gmail.com on 28 Oct 2010 at 7:58

GoogleCodeExporter commented 9 years ago 
Sounds good to me. Don't worry about backwards compatibility, you'd be doing 
the right thing.

Original comment by widd...@google.com on 28 Oct 2010 at 8:13

GoogleCodeExporter commented 9 years ago 
Checked in changes in TermVectorsFromLucene and TermTermVectorsFromLucene to 
add new constructors which do not silently use parameters from Flags.
Checked in changes to BuildBilingualIndex, BuildIndex, and BuildPositionalIndex 
to use the new constructors.

Original comment by johann.petrak@gmail.com on 25 Nov 2010 at 10:23

GoogleCodeExporter commented 9 years ago 
Thanks so much, Johann. Looks good to me.

Original comment by widd...@google.com on 27 Nov 2010 at 12:50

GoogleCodeExporter commented 9 years ago 
Unfortunately this proved unwieldy because it became too easy to create 
incompatible vectors and vector stores. Reverted to using the Flags.dimension 
global value as far as possible, except in the new vectors package itself. 
However, this is unfortunately leading to problems of its own. Comments and 
suggestions welcome.

Original comment by widd...@google.com on 25 Jul 2011 at 2:37

GoogleCodeExporter commented 9 years ago 
Changed back again to pass vectortype and dimension in and try to use them as 
little as possible in internal classes. Pretty much done and tested and 
verified, but I still think there is much room for improvement in our 
configuration approach.

Original comment by widd...@google.com on 26 Aug 2011 at 7:03