Make data saver accept numpy type floats/ints #1225
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rting them to floating points.
Codecov Report
@@ Coverage Diff @@
## master #1225 +/- ##
==========================================
+ Coverage 80.53% 80.54% +0.01%
==========================================
Files 49 49
Lines 6816 6821 +5
==========================================
+ Hits 5489 5494 +5
Misses 1327 1327 |
astafan8
reviewed
Aug 9, 2018
2) Typehint change in add_result of the datasaver class: Add that numpy types are acceptable
|
@WilliamHPNielsen @astafan8 I have added test. Can we have another look |
astafan8
approved these changes
Aug 13, 2018
|
@WilliamHPNielsen If you approve this, I will merge right away |
astafan8
reviewed
Aug 13, 2018
| data_saver = DataSaver( | ||
| dataset=test_set, write_period=0, parameters={"p": p}) | ||
|
|
||
| data_saver.add_result(("p", "some text")) |
There was a problem hiding this comment.
don't want to be annoying but:
- pep8 required newline at the end of the file
- should the test also assert that
some textis indeed in the database?
There was a problem hiding this comment.
I am quite surprised this commit even passed the quality inspection: Doesn't CI check for PEP8?
Anyway, I will correct the above.
giulioungaretti
pushed a commit
that referenced
this pull request
Aug 15, 2018
Merge: 551620d 24dfa0b Author: sohail chatoor <[email protected]> Merge pull request #1225 from sohailc/bug/1224
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1224
We test if a value is a "non-array-type" by our ability to recast it to a float. This is more robust than having a list of acceptable types as we can always miss types.