Add fetch_capture_waveform() fancy wrapper #1096

texasaggie97-zz · 2019-11-06T17:23:41Z

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?

Create a "fancy" wrapper around fetch_capture_waveform_u32()
- Returns array of named tuples Measurement(data, site)
- Gets site list and uses that when creating named tuple

List issues fixed by this Pull Request below, if any.

Fixes fetch_capture_waveform_u32 missing in nidigital API #1076

What testing has been done?

Visual inspection

…he expression

* They break codegen with 'library-only' filtering * Will be removed next time metadata updated anyway

…metadata updated to no longer have 'codegen-method': 'no' function information

…ture_waveform_u32

codecov · 2019-11-06T17:24:23Z

Codecov Report

Merging #1096 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1096   +/-   ##
=======================================
  Coverage   89.99%   89.99%           
=======================================
  Files          20       20           
  Lines        3679     3679           
=======================================
  Hits         3311     3311           
  Misses        368      368

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 004c7ae...2250729. Read the comment docs.

src/nifgen/metadata/functions.py

generated/nidigital/session.py

src/nidigital/metadata/functions_addon.py

docs/nidigital/class.rst

CHANGELOG.md

…ture_waveform_u32

marcoskirsch

There is a lot of diff noise in this PR.
I'd like to get rid of unrelated changes, and ideally do the metadata updates in their own earlier PR. Thanks!

docs/nidcpower/class.rst

docs/nidigital/class.rst

marcoskirsch · 2019-11-12T15:32:59Z

docs/nidigital/class.rst

+            - **site** (int)
+            - **data** (array.array of int)


I wonder if named tuple of int/array of int is the best way to return this.
What are all the places in the API in which this type could be used? Can it ever be an input?

It's used in two places:

niDigital_SortSiteResultsViUInt32Waveform - This will NOT be added in Python, since its use is superseded by multi-instrument sessions.

niDigital_WriteSourceWaveformSiteUniqueU32 - This should use the same type as what we use here for fetch_capture_waveform. Tracked here.

I looked at one of the use-cases for fetch_capture_waveform - streaming capture waveform data from h/w test program. It would be ideal for fetch_capture_waveform to accept a buffer and populate that with the waveform data (like fetch_into in niscope).

But the streaming use-case is not common and even then the size is about tens of MBs. If we learn about a use-case where performance is not sufficient, we can add fetch_into later. Usability of streaming with fetch_capture_waveform that returns a dict is not bad:

# Pattern burst is configured to fetch 1024 samples num_samples_expected = 16 samples_per_fetch = 4 waveforms = collections.defaultdict(list) for i in range(num_samples_expected // samples_per_fetch): fetched_waveform = multi_instrument_session.fetch_capture_waveform(site_list='', waveform_name='capt_wfm', samples_to_read=samples_per_fetch, timeout=10.0) for key in fetched_waveform: waveforms[key] += fetched_waveform[key]

So, now the question is whether to use dict or namedtuple..

One drawback for dict is iteration order is not deterministic (at least in Python 3.5). It looks like in 3.6 and later, iteration is ordered. Either way, I think it's better to not guarantee any order, because trying to do that is little tricky:

If method is called with site='' then the order will 0,1,2... with disabled sites skipped.
If method is called with site='3,0,1' (crazy but allowed), then the order will be 3,0,1 with disabled sites skipped.

Given that order depends on input and also can contain gaps, users should not really rely on order. Even if they want to iterate in a particular order, it's not difficult to do that in Python (Everything's easy in Python )

In summary, the recommendation is dict(int, array of unsigned int)

marcoskirsch · 2019-11-12T15:34:04Z

docs/nidigital/class.rst

+            :return:
+
+
+                    List of named tuples with fields:


Is the order important?
Maybe it would be nicer for clients to return a dictionary of site -> data?

See answer above.

TL;DR - dict is better.

marcoskirsch · 2019-11-12T15:35:13Z

docs/nidigital/class.rst

@@ -2114,7 +2174,7 @@ get_site_results_site_numbers



-            :type site_result_type: int
+            :type site_result_type: :py:data:`nidigital.SiteResult`


This does not appear to be related to this PR.

marcoskirsch · 2019-11-12T15:35:30Z

docs/nidmm/class.rst

@@ -1543,16 +1535,14 @@ perform_short_cable_comp
            Performs the short cable compensation measurements for the current
            capacitance/inductance range, and returns short cable compensation
            **Resistance** and **Reactance** values. You can use the return values
-            of this method as inputs to :py:meth:`nidmm.Session.ConfigureShortCableCompValues`.
+            of this method as inputs to :py:meth:`nidmm.Session.configure_short_cable_comp_values`.


These does not appear to be related to this PR.

marcoskirsch · 2019-11-12T15:35:57Z

docs/niswitch/class.rst

@@ -371,12 +371,10 @@ close
            deallocates any memory resources the driver uses. Notes: (1) You must
            unlock the session before calling :py:meth:`niswitch.Session._close`. (2) After calling
            :py:meth:`niswitch.Session._close`, you cannot use the instrument driver again until you
-            call :py:meth:`niswitch.Session.init` or :py:meth:`niswitch.Session.InitWithOptions`.
+            call :py:meth:`niswitch.Session.init` or :py:meth:`niswitch.Session.init_with_options`.


This does not appear to be related to this PR.

generated/nidigital/session.py

marcoskirsch · 2019-11-12T15:37:55Z

generated/nidigital/session.py

+
+        waveforms = []
+
+        if sys.version_info.major >= 3:


@MrDuff8 when can we drop Python 2.7?

texasaggie97-zz · 2019-11-12T16:00:38Z

I have moved this to a new PR to no longer include extraneous changes.

See #1112

texasaggie97-zz added 15 commits November 4, 2019 14:21

Initial fetch_capture_waveform()

2709431

Set library function to library-only

77db9ae

Need something to avoid flake8 error

f65cd59

ivi-dance[-with-a-twist] does not support more than one variable in t…

0b5473e

…he expression

Remove all 'codegen-method': 'no' functions

c4edad6

* They break codegen with 'library-only' filtering * Will be removed next time metadata updated anyway

Add support for 'codegen-method': 'library-only'

0ea1cb6

Remove problematic function - will be added back in a later PR

2f95ea5

Update generated files - links will go back to not existing when all …

d19768b

…metadata updated to no longer have 'codegen-method': 'no' function information

Add use_array: True

286b698

Merge branch 'codegen-method-library-only' into bug1076/add_fetch_cap…

bd308c3

…ture_waveform_u32

Update fancy_fetch_capture_waveform.py.mako

5c39cc2

Add enum so we can use it

3b4315a

Update fancy_fetch_capture_waveform.py.mako

878cf89

Update generated files

2029662

Update CHANGELOG.md

0f55cbe

sbethur reviewed Nov 6, 2019

View reviewed changes

src/nifgen/metadata/functions.py Outdated Show resolved Hide resolved

Merge branch 'master' into bug1076/add_fetch_capture_waveform_u32

151b667

texasaggie97-zz changed the title ~~Add fetch_capture_waveform() fancy wrapper~~ [#1086] Add fetch_capture_waveform() fancy wrapper Nov 6, 2019

sbethur reviewed Nov 6, 2019

View reviewed changes

generated/nidigital/session.py Show resolved Hide resolved

sbethur suggested changes Nov 6, 2019

View reviewed changes

texasaggie97-zz added 7 commits November 6, 2019 17:05

Fix name and order

ebb4474

Convert timeout from datetime before passing to private function

36443c8

Rename namedtuple and reorder items

9e29d92

Add documentation

ea2a318

Remove incorrect documentation

e275ea7

Add metadata so that generated documentation will be correct

9ca0d06

Update generated files

c841828

sbethur approved these changes Nov 6, 2019

View reviewed changes

sbethur mentioned this pull request Nov 7, 2019

nidigital fetch_capture_waveform_u32 returns empty list #1074

Closed

texasaggie97-zz added 2 commits November 7, 2019 10:31

Merge branch 'master' into codegen-method-library-only

d342658

Merge branch 'codegen-method-library-only' into bug1076/add_fetch_cap…

a42fdfd

…ture_waveform_u32

texasaggie97-zz mentioned this pull request Nov 11, 2019

Remove type from method name write_source_waveform_broadcast_u32 #1105

Closed

Merge branch 'master' into bug1076/add_fetch_capture_waveform_u32

2250729

texasaggie97-zz changed the title ~~[#1086] Add fetch_capture_waveform() fancy wrapper~~ Add fetch_capture_waveform() fancy wrapper Nov 11, 2019

marcoskirsch requested changes Nov 12, 2019

View reviewed changes

texasaggie97-zz mentioned this pull request Nov 12, 2019

Add fancy fetch capture waveform #1112

Merged

3 tasks

texasaggie97-zz closed this Nov 12, 2019

texasaggie97-zz deleted the bug1076/add_fetch_capture_waveform_u32 branch November 13, 2019 18:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fetch_capture_waveform() fancy wrapper #1096

Add fetch_capture_waveform() fancy wrapper #1096

texasaggie97-zz commented Nov 6, 2019

codecov bot commented Nov 6, 2019 •

edited

Loading

marcoskirsch left a comment

marcoskirsch Nov 12, 2019

texasaggie97-zz Nov 12, 2019

sbethur Nov 13, 2019

marcoskirsch Nov 12, 2019

texasaggie97-zz Nov 12, 2019

sbethur Nov 13, 2019

marcoskirsch Nov 12, 2019

marcoskirsch Nov 12, 2019

marcoskirsch Nov 12, 2019

marcoskirsch Nov 12, 2019

texasaggie97-zz commented Nov 12, 2019

		@@ -2114,7 +2174,7 @@ get_site_results_site_numbers



		:type site_result_type: int
		:type site_result_type: :py:data:`nidigital.SiteResult`

Add fetch_capture_waveform() fancy wrapper #1096

Add fetch_capture_waveform() fancy wrapper #1096

Conversation

texasaggie97-zz commented Nov 6, 2019

What does this Pull Request accomplish?

List issues fixed by this Pull Request below, if any.

What testing has been done?

codecov bot commented Nov 6, 2019 • edited Loading

Codecov Report

marcoskirsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

texasaggie97-zz commented Nov 12, 2019

codecov bot commented Nov 6, 2019 •

edited

Loading