search

fozziethebeat / S-Space

The S-Space repsitory, from the AIrhead-Research group
GNU General Public License v2.0
203 stars 106 forks source link

LatentSemanticAnalysis: make clear which configurations are applied when #77

Open johann-petrak opened 7 years ago

johann-petrak commented 7 years ago

The current JavaDoc states that the properties influence the creation of the transformed space when specified at processSpace time, but currently, the properties get ignored and only the settings specified at instance creation time get used. I think the most generic way to deal with this may be to use a properties instance for both the constructor (optionally, otherwise use defaults) and the processSpace method for all types of spaces. That way any behaviour before processSpace is done can be influenced by a generic properties object from the constructor and any settings in the properties object for processSpace can be used for the processing step. If a different properties object is used for processSpace, the settings there should override the ones from the object used in the constructor.

johann-petrak commented 7 years ago

BTW, I have implemented part of this in my fork (https://github.com/johann-petrak/S-Space/commit/e5c233fd5e6536856d7fde7550afc807654c6d7c)

davidjurgens commented 7 years ago

Hi Johann,

This looks much cleaner than what we had and fixes the issue. I'm happy to integrate this into the trunk if you want!

Thanks, David

On Sat, Apr 1, 2017 at 7:53 AM, Johann Petrak notifications@github.com wrote:

BTW, I have implemented part of this in my fork (johann-petrak@e5c233f https://github.com/johann-petrak/S-Space/commit/e5c233fd5e6536856d7fde7550afc807654c6d7c )

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fozziethebeat/S-Space/issues/77#issuecomment-290924933, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7uLUv6-1V2vZqXZ_hVzVgcmnJ3KTK0ks5rrmT3gaJpZM4MvwQB .

johann-petrak commented 7 years ago

If you like the suggestion I can implement all of it and provide a pull request for this. I am also happy with you copying over the version is it is now, whatever you prefer.