-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add registry for numerical type size and endianness #17
base: master
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
@@ -93,6 +84,117 @@ numeric_##type##_to_##gtype (numeric_##type num) \ | |||
return data.v_##gtype; \ | |||
} | |||
|
|||
GArray *TYPE_TABLE = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.