Session.__init__() can take dict for the option parameter#723
Session.__init__() can take dict for the option parameter#723texasaggie97-zz merged 55 commits intomasterfrom
Conversation
…han just timeouts
…o something different for the name and setup to run the converter
texasaggie97-zz
changed the title
texasaggie97-zz
changed the title
texasaggie97-zz
dismissed
marcoskirsch’s stale review
Review comments addressed
marcoskirsch
left a comment
marcoskirsch
left a comment
There was a problem hiding this comment.
I think the example changes make sense.
marcoskirsch
left a comment
marcoskirsch
left a comment
There was a problem hiding this comment.
I will do another full pass after #718 is merged, since as-is this shows 50 files changed including things related to repeated capabilities.
| * ### ALL | ||
| * #### Added | ||
| * #### Changed | ||
| * Option string can now be a python dictionary instead of a string. It will be converted as needed (Fix #661) |
There was a problem hiding this comment.
- Use markdown to link to Improve Session constructor API by replacing option_string with something more Pythonic #661 (we should make that standard practice)
- We should move detailed documentation to the appropriate places (RST / docstring)
There was a problem hiding this comment.
It is also added to docstrings. It can be added to rst/readthedocs once #722 is merged
| * Option string can now be a python dictionary instead of a string. It will be converted as needed (Fix #661) | ||
| * Key/Value pairs approporiate for desired behavior | ||
| ``` python | ||
| session = nidmm.Session('Dev1', False, {'Simulate': True, 'DriverSetup': {'Model': '4071', 'BoardType': 'PXI'}}) |
There was a problem hiding this comment.
Prefer Python convention for dictionary terms. (we settled on that right?)
There was a problem hiding this comment.
Changed to pythonic spelling
| * #### Added | ||
| * #### Changed | ||
| * `nidmm.Session()` no longer takes id_query parameter | ||
| * Metadata updated to NI-DMM 17.5 |
There was a problem hiding this comment.
Why does this show up in diff? Is this not updated with latest master?
There was a problem hiding this comment.
It is updated with master. Not sure why it is in the diff
texasaggie97-zz
dismissed
marcoskirsch’s stale review
Review comments addressed
marcoskirsch
changed the title
| commands.log | ||
| .vscode/launch.json | ||
| .vscode/settings.json | ||
| .pytest_cache/ |
There was a problem hiding this comment.
integration mistake, this is now in here twice.
There was a problem hiding this comment.
One copy removed
| You can specify a subset of repeated capabilities using the Python index notation on an | ||
| {0}.Session instance, and calling set/get value on the result.: | ||
|
|
||
| session['0,1'].{0} = var |
There was a problem hiding this comment.
This is not how things are handled anymore in master.
marcoskirsch
left a comment
marcoskirsch
left a comment
There was a problem hiding this comment.
There's enough complexity around generating the init() methods, that I'm starting to wonder if it would have been simpler to just hand-code them.
| return _convert_timedelta(value, library_type, 1000000) | ||
|
|
||
|
|
||
| # This converter is not called from the normal codegen path for function. Instead it is |
There was a problem hiding this comment.
Why not put this as part of the docstring?
There was a problem hiding this comment.
Because I didn't think the information in the comment was appropriate for a docstring.
I can move into one if you think it is.
|
test this please |
1 similar comment
|
test this please |
2 participants
What does this Pull Request accomplish?
List issues fixed by this Pull Request below, if any.
What testing has been done?