cazala / synaptic

architecture-free neural network library for node.js and the browser
http://caza.la/synaptic
Other
6.92k stars 666 forks source link

Misleading parameter names in Network.toJSON() and Network.fromJSON() #172

Closed stjepangolemac closed 7 years ago

stjepangolemac commented 7 years ago

Names of parameters in those two functions are misleading in a way that they should not be JSON strings but objects. Moreover, I am using a new version of Typescript with Visual Studio Code and it automatically resolves type definitions that define the same parameters as strings.

This is not a critical problem but it confused me and made me search for a solution for some time.

Jabher commented 7 years ago

so your syntax highlight is expecting toJSON(): string and fromJSON(json:string)?

stjepangolemac commented 7 years ago

Yes, the type definitions define those two functions as you wrote. For exporting to work I must do it like this: let exported = JSON.stringify(myNetwork.toJSON()); let imported = Network.fromJSON(JSON.parse(exported));

Is this the expected way of exporting or not? Even without type definitions in your source file parameter is named json so one would assume it is a string type.

Jabher commented 7 years ago

it is intended behavior. .toJSON can return anything serializable, it should not return already serialized value, see docs: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#toJSON()_behavior

stjepangolemac commented 7 years ago

It makes sense now. Thanks.