Skip to content
This repository was archived by the owner on Feb 26, 2025. It is now read-only.

Conversation

@WeinaJi
Copy link

@WeinaJi WeinaJi commented Mar 3, 2023

Description
Add support for std::byte as Dataset type when using c++17 and above

  • Issue 1 fixed
  • Feature 1 added

Fixes #480

How to test this?

cmake -DCMAKE_CXX_STANDARD=17 ..
make -j8
make test

Test System

  • OS: [e.g. Ubuntu 20.04]
  • Compiler: [e.g. clang 12.0.0]
  • Dependency versions: [e.g. hdf5 1.12]

@codecov-commenter
Copy link

codecov-commenter commented Mar 3, 2023

Codecov Report

Merging #698 (7062851) into master (20be06f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #698   +/-   ##
=======================================
  Coverage   81.37%   81.37%           
=======================================
  Files          67       67           
  Lines        4248     4248           
=======================================
  Hits         3457     3457           
  Misses        791      791           
Impacted Files Coverage Δ
include/highfive/bits/H5DataType_misc.hpp 78.64% <ø> (ø)
tests/unit/test_all_types.cpp 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

alkino
alkino previously approved these changes Mar 4, 2023
@WeinaJi WeinaJi requested a review from 1uc March 4, 2023 19:26
1uc
1uc previously approved these changes Mar 6, 2023
Copy link
Collaborator

@1uc 1uc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks!

DATASET_NAME, {5}, create_datatype<typename details::inspector<TestType>::base_type>());

// Write into the initial part of the dataset
dataset.write(t1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite the above tests not doing it, it might be nice to also check:

        auto dataset2 = file.createDataSet(DATASET_NAME2, t1);

We could leave this for when we refactor the whole file to be more DRY.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I leave this for the refactoring later.

@WeinaJi WeinaJi dismissed stale reviews from 1uc and alkino via 7062851 March 6, 2023 09:39
@1uc 1uc merged commit c6ffef4 into master Mar 6, 2023
@1uc 1uc deleted the weji/byte_c++17 branch March 6, 2023 15:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for std::byte as Dataset type

5 participants