Time extension #91
Time extension #91
Conversation
* Use asdf_time_info_t structure member
| ASDF_TIME_FORMAT_PLOT_DATE, | ||
| ASDF_TIME_FORMAT_YMDHMS, | ||
| ASDF_TIME_FORMAT_datetime64, | ||
| } asdf_time_base_format; |
There was a problem hiding this comment.
I like to add the _t suffix even with type-def'd enums. But I don't have a strict rule about this. Just mentioning it but I won't nitpick over it beyond that ;)
| struct asdf_time_info_t { | ||
| struct timespec ts; | ||
| struct tm tm; | ||
| }; |
There was a problem hiding this comment.
Ah, but then don't use the _t suffix for anything that isn't typedef'd; then it's hard to remember it needs struct :)
| ASDF_LOCAL int asdf_time_parse_std(const char *s, const asdf_time_format_t *format, struct asdf_time_info_t *out); | ||
| ASDF_LOCAL int asdf_time_parse_byear(const char *s, struct asdf_time_info_t *out); | ||
| ASDF_LOCAL int asdf_time_parse_yday(const char *s, struct asdf_time_info_t *out); | ||
| ASDF_LOCAL void show_asdf_time_info(const struct asdf_time_info_t *t); |
There was a problem hiding this comment.
Another naming nitpick, especially for anything with non-static linkage--should be asdf_time_show_info. Basically asdf_<domain>_<verb> like you did with the other ones.
| int days[] = {31,28,31,30,31,30,31,31,30,31,30,31}; | ||
| if (month == 2 && is_leap_year(year)) return 29; | ||
| return days[month - 1]; | ||
| } |
There was a problem hiding this comment.
These should be declared static
| const int HOURS_PER_DAY = 24; | ||
| const int SECONDS_PER_DAY = 86400; | ||
| const int SECONDS_PER_HOUR = 3600; | ||
| const int SECONDS_PER_MINUTE = 60; |
There was a problem hiding this comment.
More stuff should be static around here.
| + day_of_month + fractional_day + gregorian_correction - JD_EPOCH_SHIFT; | ||
|
|
||
| return julian_date; | ||
| } |
There was a problem hiding this comment.
Very impressive--I never would have felt like writing this, though I think I've written it in Python before. It all looks correct to me but it's easy to get lost with this stuff.
| "t_jd", | ||
| "t_mjd", | ||
| "t_byear", | ||
| }; |
There was a problem hiding this comment.
If you want you can also write parametrized tests with μnit. Not so important here but you can see an example in test-ndarray.c for example (test_numeric_conversion_params).
| #include "../log.h" | ||
| #include "../util.h" | ||
| #include "../value.h" | ||
| #include "stc/cregex.h" |
There was a problem hiding this comment.
I haven't succumbed to the temptation to use STC's regex library yet, but this is an obvious good use for it, and it's there, so might as well. You'll probably need to add it here
libasdf/third_party/Makefile.am
Line 6 in defb334
make distcheck works.
|
|
||
| value = asdf_get_value(file, key); | ||
| if (asdf_value_as_time(value, &t) != ASDF_VALUE_OK) { | ||
| fprintf(stderr, "asdf_value_as_time failed: %s\n", key); |
There was a problem hiding this comment.
Oh yeah, you can use
munit_logf("asdf_value_as_time failed: %s\n", key);for this and other print calls in the tests. And below this you can return MUNIT_FAIL
2 participants
No description provided.