Skip to content
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

Infer dtype from data when spec dtype is none or numeric #143

Merged
merged 6 commits into from
Sep 26, 2019
Merged

Conversation

rly
Copy link
Contributor

@rly rly commented Sep 25, 2019

Fixes #140 and fixes #142.

If an AbstractDataChunkIterator is used for a dataset that has the dtype 'numeric' or no dtype in the spec, then the dtype used to write the dataset will be inferred from the dtype of the AbstractDataChunkIterator. Same for all numpy types, lists and tuples, and Python primitives. Previously, the dtype used to write the dataset was set to None, and so I believe h5py used a default dtype of float32.

@codecov
Copy link

codecov bot commented Sep 25, 2019

Codecov Report

Merging #143 into dev will increase coverage by 0.2%.
The diff coverage is 66.66%.

Impacted file tree graph

@@          Coverage Diff           @@
##            dev    #143     +/-   ##
======================================
+ Coverage    69%   69.2%   +0.2%     
======================================
  Files        24      24             
  Lines      4971    4981     +10     
  Branches   1139    1143      +4     
======================================
+ Hits       3430    3447     +17     
+ Misses     1172    1168      -4     
+ Partials    369     366      -3
Impacted Files Coverage Δ
src/hdmf/spec/spec.py 66.81% <0%> (+0.14%) ⬆️
src/hdmf/backends/hdf5/h5tools.py 63.22% <33.33%> (-0.2%) ⬇️
src/hdmf/build/map.py 60.85% <78.57%> (+1.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 709f1f9...4bf54a6. Read the comment docs.

@rly
Copy link
Contributor Author

rly commented Sep 25, 2019

I still need to add tests, but just wanted to put the proposed solution out there.

@luiztauffer
Copy link

I tested on my code and this PR solved the problem, related to #140 , thanks!

Copy link
Contributor

@oruebel oruebel left a comment

Choose a reason for hiding this comment

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

Looks good to me.

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.

'numeric' dtype in spec acts as None dtype DataChunkIterator converting data to float
4 participants