-
Notifications
You must be signed in to change notification settings - Fork 87
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
NI-DMM: Make getting the last calibration temp and datetime consistent with other drivers. #1498
base: master
Are you sure you want to change the base?
Conversation
…nctions private, or removed them altogether in the case of the get last cal date and time one, which is now redundant and unnecessary.
Can one of the admins verify this patch? |
Codecov Report
@@ Coverage Diff @@
## master #1498 +/- ##
==========================================
+ Coverage 91.65% 93.88% +2.22%
==========================================
Files 20 32 +12
Lines 3451 6848 +3397
==========================================
+ Hits 3163 6429 +3266
- Misses 288 419 +131
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
ok to test |
I'll leave the branch on my fork of the repo so that any code I wrote doesn't just disappear. If I'd had a week more, my plan was to look into making the cal temp/date and time stuff into single-sourced templates, but I don't think that's feasible in the span of approximately a day since I've never looked into that part of the codebase at all before. |
I think it's worth adding these methods, and we worry about obsoleting the old inconsistent ones separately. Do you agree @sbethur ? |
I agree. But I'd want to add it without duplicating the mako templates between |
This contribution adheres to CONTRIBUTING.md.
I've updated CHANGELOG.md if applicable.
I've added tests applicable for this pull request
What does this Pull Request accomplish?
Adds external and self calibration variations of get last cal temp and get last cal date and time, in order to be consistent with other drivers with similar methods. Removes the previously existing get last cal date time method since it was an added function rather than part of base metadata, and is now redundant with the newly added methods.
List issues fixed by this Pull Request below, if any.
nidmm.Session.get_cal_date_and_time
andget_last_cal_temp
is different from other nimi-python drivers #1462What testing has been done?
Branch has successfully built and passed all system tests on a local testing machine.