search

own-pt / glosstag

Semantically Tagged PWN glosses
Other
7 stars 4 forks source link

export Mongo json to TSV #15

Closed arademaker closed 5 years ago

arademaker commented 5 years ago

https://github.com/own-pt/glosstag/blob/master/src/export.lisp#L19

Is it a bug or we are working on diff data formats? This is a so silly bug that I need to make sure if I am not using a diff version of the data format in my sensetion installation.

arademaker commented 5 years ago

Otherwise, the problem is here

https://github.com/own-pt/glosstag/blob/master/src/data.lisp#L36-L37

@odanoburu can you explain? Not asking to solve the problem, just to explain what is going on...

odanoburu commented 5 years ago

I don't follow what the problem is, can you explain? I don't see any problem with line 19. remember I changed the architecture to parse from a concrete format (in this case JSON) to an abstract representation (the common lisp structs for sentences and tokens). the abstract representation is validated, and then it is exported to another concrete representation (in this case TSV).

arademaker commented 5 years ago

Actually, the error that was raised was https://github.com/own-pt/glosstag/blob/master/src/data.lisp#L49-L50. So the error is a missing glob field (that encodes how the glob was created). I initially thought that glob field is the glob id..

arademaker commented 5 years ago

The existence of this missing glob field is a problem, it was not created by the sensetion during the creation of a glob. The name of the field is also not good at all.

odanoburu commented 5 years ago

I initially thought that glob field is the glob id..

The name of the field is also not good at all.

yes, it's pretty confusing, following princeton names was not a good idea here

The existence of this missing glob field is a problem, it was not created by the sensetion during the creation of a glob.

I think this was an old bug that got corrected (see https://github.com/own-pt/sensetion.el/blob/86246ec71be1117e78340acbcf7717f91b190a18/sensetion.el#L512-L513), but we probably didn't correct the globs created with this bug?