Add registry for numerical type size and endianness #17
Conversation
|
I suggest making name a fixed size array.
These are scalar types, so I dont think we want to add vector size or
composite types here.
There is a standard byte order swapping algorithm, e.g.
https://github.com/rainwoodman/bigfile/blob/41623783e61a6971bb1ab4a09cc3d797b6d90283/src/bigfile.c#L1297
I think I took that from numpy.
…On Thu, Feb 13, 2020, 1:34 PM Guillaume Poirier-Morency < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/numeric-types.c
<#17 (comment)>:
> @@ -93,6 +83,102 @@ numeric_##type##_to_##gtype (numeric_##type num) \
return data.v_##gtype; \
}
+typedef struct _NumericTypeInfo
+{
+ GType type;
+ const gchar *name;
+ gsize width;
+ NumericByteOrder byte_order;
@rainwoodman <https://github.com/rainwoodman> If you can think of any
other field that could be of use in Vast for example.
If we know the width and byte ordering, we could very likely generate the
code for byte swapping.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#17?email_source=notifications&email_token=AABBWTEHZJPVA7UWKI5DAYLRCW4F5A5CNFSM4KU2OG6KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCVPU3EI#pullrequestreview-358567313>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABBWTE6NE6JFRXL6LZOA2TRCW4F5ANCNFSM4KU2OG6A>
.
|
Use G_DEFINE_BOXED_TYPE_WITH_CODE to register numeric typeinfo counterpart lazily in the *_get_type routine. Reuse GLib byte order macros since our definitions are at best redundant.
src/numeric-types.c
Outdated
| numeric_type_register_static (G_TYPE_INT64, "int64", sizeof (gint64), G_BYTE_ORDER); | ||
| numeric_type_register_static (G_TYPE_UINT64, "uint64", sizeof (guint64), G_BYTE_ORDER); | ||
| numeric_type_register_static (G_TYPE_FLOAT, "float", sizeof (gfloat), G_BYTE_ORDER); | ||
| numeric_type_register_static (G_TYPE_DOUBLE, "double", sizeof (gdouble), G_BYTE_ORDER); |
There was a problem hiding this comment.
The issue here is that we cannot redefine these types. We can either have a numeric boxed counterpart or look into switching from boxed types to fundamental types for our definitions.
The quickfix would be to define these "fundamental" types lazily in numeric_get_type_info.
There was a problem hiding this comment.
2 quick points: Would life be made easier if not using macros but hard code and/or generate the definitions with a code generator?
I remember the get_type() functions always generate the type the first time. Is that what you gain by moving to a fundamental type? If adding a fundamental type. I would suggest considering GNumericScalar as an umbrella type for all of the types.
There was a problem hiding this comment.
It's actually doing just that! G_DEFINE_BOXED_TYPE_WITH_CODE inserts arbitrary code inside the _get_type definition allowing us to perform any kind of initialization and registration of transformations.
The issue I'm facing with GLib fundamental types is that we cannot redefine that piece of code and thus registering the NumericTypeInfo counterpart we will need for introspection and byteswapping.
src/numeric-types.c
Outdated
| return data.v_##gtype; \ | ||
| } | ||
|
|
||
| GArray *TYPE_TABLE = NULL; |
There was a problem hiding this comment.
Make this declaration static.
Let numeric_type_register_static take the NumericTypeInfo struct as argument. Move registration of numeric counterparts of GLib fundamental types into numeric_get_type_info(). Mark the TYPE_TABLE symbol as static. Remove _get_name, _get_type and _get_byte_order in favour of using the now public NumericTypeInfo struct members directly.
| numeric_type_register_static_simple (G_TYPE_UINT64, "uint64", sizeof (guint64), G_BYTE_ORDER); | ||
| numeric_type_register_static_simple (G_TYPE_FLOAT, "float", sizeof (gfloat), G_BYTE_ORDER); | ||
| numeric_type_register_static_simple (G_TYPE_DOUBLE, "double", sizeof (gdouble), G_BYTE_ORDER); | ||
| } |
There was a problem hiding this comment.
It would be great if we could enumerate these types at runtime so that the we could be forward compatibly if new types are defined in GLib.
| } | ||
|
|
||
| public unowned Numeric.TypeInfo get_type_info (GLib.Type type); | ||
| public unowned Numeric.TypeInfo get_type_info_from_name (string name); |
There was a problem hiding this comment.
These functions should be defined within the TypeInfo class as static.
| DEFINE_NUMERIC_WITH_BYTESWAP (float_le, G_LITTLE_ENDIAN, float, gint32, GINT32, LE) | ||
| DEFINE_NUMERIC_WITH_BYTESWAP (float_be, G_BIG_ENDIAN, float, gint32, GINT32, BE) | ||
| DEFINE_NUMERIC_WITH_BYTESWAP (double_le, G_LITTLE_ENDIAN, double, gint32, GINT64, LE) | ||
| DEFINE_NUMERIC_WITH_BYTESWAP (double_be, G_BIG_ENDIAN, double, gint32, GINT64, BE) |
There was a problem hiding this comment.
I think we're almost good now. I will incorporate your piece of code (if license permit?) to perform generic byte swapping.
There was a problem hiding this comment.
No problem. There is a minimal version of numpy 'dtype' in bigfile, included some macros for casting and striding. Feel free to recycle any pieces there.
It will be useful later on when we integrate dynamic types.
No description provided.