Skip to content

Conversation

aldanor
Copy link
Owner

@aldanor aldanor commented Jun 10, 2023

Closes #236 (?)

@aldanor aldanor requested a review from mulimoen June 10, 2023 23:16
@aldanor
Copy link
Owner Author

aldanor commented Jun 10, 2023

  • We now run roundtrip tests for both f16 and complex
  • Refactored DynValue in this PR as well, packing floats into one variant, now that there's (up to) 3 of them
  • "complex" and "f16" are now top-level features in the main crate
  • Doc builds now have f16/complex enabled
  • Enabled f16/complex in at least one build, the static one (features in the CI are a complete mess...)

@aldanor aldanor changed the title Experimental float16 support Float16 support Jun 10, 2023
@aldanor
Copy link
Owner Author

aldanor commented Jun 11, 2023

@mulimoen In the failures, your test_issue_223 test has failed apparently 🤔

(btw I moved the tests under hdf5/ now, so previously things might have been running slightly incorrectly...)

Copy link
Collaborator

@mulimoen mulimoen left a comment

Choose a reason for hiding this comment

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

With the small nits and #247 merged this looks great!

@aldanor
Copy link
Owner Author

aldanor commented Jun 11, 2023

Enabled f16/complex for docs.rs builds for both hdf5 and hdf5-types - thanks, good call.

As you said above, exhaustive attr might not be the best idea since it would force ourselves to panic when matching which is a bit weird.

If it builds green, I think it should be good to go :)

@mulimoen
Copy link
Collaborator

Looks good to go if green!

@aldanor aldanor merged commit 26046fb into master Jun 11, 2023
@aldanor aldanor deleted the feature/f16 branch June 11, 2023 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slow performance compared to h5py
2 participants