search

llohse / libnpy

C++ library for reading and writing of numpy's .npy files
MIT License
361 stars 72 forks source link

Add github ci test pipeline. #17

Closed caic99 closed 2 years ago

caic99 commented 2 years ago

Using the provided tests/Makefile as testing routine.

llohse commented 2 years ago

Thank you for this PR. This reminds me that I had planned to add some automated tests for a long time. I will look into it tomorrow.

caic99 commented 2 years ago

Thank you for this PR. This reminds me that I had planned to add some automated tests for a long time. I will look into it tomorrow.

In fact, we are facing a problem using libnpy with gcc-10 in a large project.

If the header is included by several files it results in multiple definitions of the same variable.

The new "feature"(Default to -fno-common) conflicts with the has_typestring part within npy.hpp. So I plan to fix it by myself.

caic99 commented 2 years ago

By the way, is it OK to put declaration and implementation all together in one .hpp file? I suggest to split them apart. @llohse

llohse commented 2 years ago

By the way, is it OK to put declaration and implementation all together in one .hpp file? I suggest to split them apart. @llohse

This is common practice for a header-only library. From my point of view, the file is also small enough to not be split into multiple headers. I could be convinced otherwise, though. Do you have a good reason to split them apart?

caic99 commented 2 years ago

By the way, is it OK to put declaration and implementation all together in one .hpp file? I suggest to split them apart. @llohse

This is common practice for a header-only library. From my point of view, the file is also small enough to not be split into multiple headers. I could be convinced otherwise, though. Do you have a good reason to split them apart?

Would you see #19 .

llohse commented 2 years ago

closing in favor of #24