-
Notifications
You must be signed in to change notification settings - Fork 13
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
Missing multiplier: add option to allow no-rescaling #47
Comments
Hi @ml-evs The risk with overriding the error with a default of 1 is you may unknowingly be returning incorrect current and capacity values. |
Yep, understood, if you're happy to keep up with new instruments then I'm happy to help! I still think an override would be useful but understand that it might prevent people from contributing back. The |
Hi @ml-evs I just pushed a commit to development with the missing current range. The file The file For reference, here is the Neware utility I use to load nda/ndax files and check that current and capacity are being scaled correctly: https://newarebattery.com/softwares/BTSDA20221226.zip |
Much appreciated, and that link looks super useful. I'll close my PR and this issue now. Thanks! |
Hi there, thanks for this useful package! I've been trying to integrate Neware support into navani (a wrapper for several packages that allows us to normalize battery data across all brands in our lab) and thus datalab (our group's data management platform).
Some of the test data I've been provided does not work with the current version of this package, as we are missing the instrument
Range=5
. I would really like to be able to use this package in a mode that is robust to missing multipliers, e.g., give a gigantic warning but do not error if the instrument scaling has not been determined, as in our case, the user can supply the desired scaling manually to be stored alongside the data file. Either this could be enabled by an option, or be the default.I've coded this up in my fork which I can continue to use for now, but thought I should open an issue in the meantime to accompany an upcoming PR. Is this functionality you would like to add?
The text was updated successfully, but these errors were encountered: