alavrik / piqi

Piqi – universal schema language: JSON, XML, Protocol Buffers data validation and conversion
http://piqi.org
Apache License 2.0
246 stars 36 forks source link

bug? #7

Closed joelreymont closed 13 years ago

joelreymont commented 13 years ago

piqi to-proto Topic.piqi Warning: Topic.piqi:1:1: unknown field

piqi to-proto Ad.piqi Warning: ./Topic.piqi:1:1: unknown field Warning: Ad.piqi:1:1: unknown field

protoc --python_out=. Topic.piqi.proto

protoc --python_out=. Ad.piqi.proto Ad.piqi.proto:6:14: "topic_probability_list" is not defined.

cat Topic.piqi .ocaml-module "Topic" .proto-package "Topic"

% probability: [0;1] .alias [ .name probability .type float ]

.list [ % A Topics/distribution-list is an LDA vector .name topic-probability-list .type probability ]

.alias [ .name vector .type topic-probability-list ]

cat Ad.piqi .ocaml-module "Ad" .proto-package "Ad"

.import [ .module Topic ]

.record [ .name datastore-entry .field [ .name topic-vector .type Topic/vector ] ]

joelreymont commented 13 years ago

It looks like what piqi should be outputting is

package Ad;

import "Topic.piqi.proto" ;

message datastore_entry { required Topic.topic_probability_list topic_vector = 1; }

instead of

package Ad;

import "Topic.piqi.proto" ;

message datastore_entry { required topic_probability_list topic_vector = 1; }

alavrik commented 13 years ago

Yep, this is a bug. It should generate fully qualified Proto type names in such case, but it is broken for some reason.

I'll fix it later today.

alavrik commented 13 years ago

As a workaround, you can specify

.record [ .name datastore-entry .field [ .name topic-vector .type Topic/topic-probability-list ] ] ]

instead of

.record [ .name datastore-entry .field [ .name topic-vector .type Topic/vector ] ]

It is essentially the same and I've just verified that this way it generates a proper fully-qualified type name.

joelreymont commented 13 years ago

Confirm that workaround of using topic-probability-list directly works. I actually ended up removing the alias.

alavrik commented 13 years ago

Fixed in "master-fixes" and "dev" branches. It should be working correctly now.

joelreymont commented 13 years ago

Awesome, thanks!