Repeated Capabilites documentation improvements #1686

ni-jfitzger · 2022-01-21T23:04:27Z

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?

There are several issues open against the way we document usage of repeated capabilities in our rep_caps.rst pages.

Summary of Issues

We present implementation details that the user doesn't care about
- These may be useful to contributors, but to everyone else they're just distracting.
We list all of the possible types that can be passed to the indexing operator but don't make it clear what the recommended or "pythonic" types are.
- This information is worth keeping, but we need to provide better guidance.
We use channel_enabled in all of our examples, though it may not actually support the repeated capability in the example code and not all apis have a channel_enabled property.

This PR addresses these by doing the following:

Explicitly recommend what types to pass to the indexing operator.
Present examples which demonstrate accessing the repeated capability with some of our recommended types.
- Do not present examples using other types.
Specify fields in metadata used to build real examples specific to each rep cap
- New helper functions were added to return the snippet and explanation for each example

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

What testing has been done?

Local code generation with documentation inspection.

There are several issues open against the way we document usage of repeated capabilites in our rep_caps.rst pages. Summary of Issues * We present implementation details that the user doesn't care about - These may be useful to contributors, but to everyone else they're just distracting. * We list all of the possible types that can be passed to the indexing operator but don't make it clear what the recommend or "python" types are. - This information is worth keeping, but we need to provide better guidance. * We use channel_enabled in all of our examples, though it may not actually support the repeated capability in the example code. The approach to fixing this is as follows: * Turn documentation related to implementation details into comments only viewable in the raw source * Explicilty recommend what types to pass to the indexing operator. * Present examples which demonstrate accessing the repeated capability with our recommended types. - Do not present examples using other types. * Add a note that channel_enabled may not actually supported the repeated capability and to check individual property and method documentation.

build/templates/rep_caps.rst.mako

ni-jfitzger · 2022-01-21T23:09:35Z

build/templates/rep_caps.rst.mako

-        passes a string of :python:`'${prefix}0, ${prefix}1, ${prefix}2'` to the set attribute function.
+            session.${name}[0:2].channel_enabled = True
+
+        sets :py:attr:`channel_enabled` to :python:`True` for ${name} 0, 1, 2.


If I add anything to the recommend types above, I should add an example usage, here.

nimi-bot · 2022-01-21T23:10:16Z

Can one of the admins verify this patch?

ni-jfitzger · 2022-01-21T23:11:30Z

I haven't committed the results of tox -e codegen.
I can do that when there's agreement on the contents of rep_caps.rst.mako.

marcoskirsch · 2022-01-24T19:12:34Z

I haven't committed the results of tox -e codegen.
I can do that when there's agreement on the contents of rep_caps.rst.mako.

No, please commit them before the PR is ready for review so we can see the effects and merge without having to track anything.

ni-jfitzger · 2022-01-24T19:20:15Z

committed the results of tox-e codegen as requested.

docs/nidcpower/rep_caps.rst

build/templates/rep_caps.rst.mako

sbethur · 2022-01-29T01:16:31Z

build/templates/rep_caps.rst.mako


-    Some repeated capabilities use a prefix before the number and this is optional
+    The recommended way of accessing repeated capabilities is with an integer :python:`[0]` or range :python:`[0:2]`.


range :python:[0:2]

I don't think this will work since this is not the correct syntax for Python range. But ['0:2'] will work because of the custom logic we have for handling range-based strings.

Having said that, I don't think any of the ways I've enumerated above can be recommended over others. int is obviously the go-to for single instances. Basic sequence types and slice are good to use because they're part of the language, Range-based strings provides syntactic sugar for defining ranges. e.g. ['0:2'] rather than [range(0,2)].

I see @marcoskirsch doesn't like us to recommend the range-based strings. Since I saw we had logic specifically for handling it, I thought it was important. But it sounds like it was ill-conceived. I don't mind not documenting it.

Once we land on recommended ways, we should go over the examples to ensure only recommended types are used.

build/templates/rep_caps.rst.mako

…nto ni-jfitzger/rep_cap_documentation-overhaul

ni-jfitzger · 2023-07-11T20:19:37Z

build/templates/rep_caps.rst.mako


-    Some repeated capabilities use a prefix before the number and this is optional
+    The recommended way of accessing multiple repeated capabilites at once is with a tuple (:python:`[0, 1]` or :python:`['Dev1', 'Dev2']`) or slice :python:`[0:2]`.


I'm not currently providing slice usage examples. Let me know if you want me to add them.

I think that would be nice.
I wonder if we should change wording to say that it should be an iterable rather than provide examples of iterables. Or at least it should be :)

Well, we already mention supported values above:

Use the indexing operator [] to indicate which repeated capability instance you are trying to access. The parameter can be a single element or an iterable that implements sequence semantics, like list, tuple, range and slice.

I guess it wouldn't hurt, to keep this part more generic, though, especially given our examples below.

ni-jfitzger · 2023-07-11T20:22:00Z

docs/nidigital/rep_caps.rst

Still undocumented: rep cap chaining.

different issue/pr?

docs/nidcpower/rep_caps.rst

marcoskirsch · 2023-07-12T15:42:14Z

docs/nidcpower/rep_caps.rst

+    Use the indexing operator :python:`[]` to indicate which repeated capability instance you are trying to access.
+    The parameter can be a single element or an iterable that implements sequence semantics, like list, tuple, range and slice.
+
+    The recommended way of accessing a single repeated capability is with an integer :python:`[0]` for capabilities that support it and a string :python:`['Dev1']`


I think all the repeated capabilities we have in nimi-python, at least at the moment, work with integers with the exception of instruments.

I wonder if we should remove this sentence, and instead where we list the repeated capabilites we tell you that the index should be an integer or an iterable of integers (or strings).

We can deduce which one it would be programmatically by looking at the metadata, specifically the rep cap prefix.

marcoskirsch · 2023-07-12T15:43:36Z

docs/nidcpower/rep_caps.rst


-        passes a string of :python:`'0, 1, 2'` to the set attribute function.
+            session.channels[0, 2].output_function = nidcpower.OutputFunction.DC_CURRENT



here is where we could add an example using an iterable, below line 42?

session.channles[range(3)].output_function = nidcpower.OutputFunction.DC_CURRENT

marcoskirsch · 2023-07-12T15:45:00Z

docs/nidcpower/rep_caps.rst


-        passes a string of :python:`'0, 1, 2'` to the set attribute function.
+            print(session.instruments['Dev1', 'Dev2', '3rdDevice'].serial_number)


I found 3rdDevice confusing. Do we even allow aliases that begin with a number?

Consider using values closer to the defaults customers would encounter:

['PXI1Slot3', 'PXI1Slot4', 'PXI1Slot5']

We do allow aliases that begin with a number, yes. They don't have to have letters, at all.

I found 3rdDevice confusing.

I just didn't want users to think it had to start with "Dev". I have the same concern with the examples you gave.
Maybe my blurb about the basic element should specify "device alias string" for instruments?

Right, just use something that matches defaults. This will be informative enough and not confused anyone.

Maybe my blurb about the basic element should specify "device alias string" for instruments?

I question how familiar customers are with term "device alias". It's certainly not in our APIs. Let's avoid,

marcoskirsch · 2023-07-12T15:46:07Z

docs/nidigital/rep_caps.rst

+    #. pattern_opcode_events_
+    #. conditional_jump_triggers_
+    #. sites_
+    #. rio_events_


I'm not sure about this... are the RIO triggers and events usable from Python at all in an application?

They are. We test it:

nimi-python/src/nidigital/system_tests/test_system_nidigital.py

Line 146 in 2986696

def test_rio_events_rep_cap(self, single_instrument_session):

nimi-python/src/nidigital/system_tests/test_system_nidigital.py

Line 153 in 2986696

def test_rio_triggers_rep_cap(self, single_instrument_session):

They work. But are they actually useful in an application? Given that there is no LVFPGA support in Python.

Oh, maybe not.
I can create an issue to remove them.

They work. But are they actually useful in an application? Given that there is no LVFPGA support in Python.

I think there is support.
https://github.com/ni/nifpga-python

src/niscope/metadata/config_addon.py

…ype in documentation for each capability.

ni-jfitzger requested review from marcoskirsch and sbethur as code owners January 21, 2022 23:04

ni-jfitzger commented Jan 21, 2022

View reviewed changes

build/templates/rep_caps.rst.mako Outdated Show resolved Hide resolved

ni-jfitzger commented Jan 21, 2022

View reviewed changes

codegen with rep_caps.rst changes

7b3a1a5

marcoskirsch requested changes Jan 24, 2022

View reviewed changes

docs/nidcpower/rep_caps.rst Show resolved Hide resolved

docs/nidcpower/rep_caps.rst Outdated Show resolved Hide resolved

docs/nidcpower/rep_caps.rst Outdated Show resolved Hide resolved

docs/nidcpower/rep_caps.rst Outdated Show resolved Hide resolved

ni-jfitzger added 2 commits January 28, 2022 12:19

Remove comment containing tips we decided weren't useful

8bc69bc

Update general description of repeated capabilities.

93d62f1

sbethur suggested changes Jan 29, 2022

View reviewed changes

sbethur reviewed Jan 31, 2022

View reviewed changes

build/templates/rep_caps.rst.mako Outdated Show resolved Hide resolved

ni-jfitzger added 14 commits April 3, 2023 09:01

Merge branch 'master' of https://github.com/ni-jfitzger/nimi-python i…

e4ba124

…nto ni-jfitzger/rep_cap_documentation-overhaul

indexing is for repeated capability **instance**

4fda89a

Remove commented section for non-0-length prefixes

948db7a

Merge branch 'master' of https://github.com/ni-jfitzger/nimi-python i…

f2dc5ea

…nto ni-jfitzger/rep_cap_documentation-overhaul

Merge branch 'master' of https://github.com/ni-jfitzger/nimi-python i…

f04652c

…nto ni-jfitzger/rep_cap_documentation-overhaul

generate correct snippets for nidigital

d48cfc9

generate correct rep cap documentation for nidcpower

1cc9cf4

generate correct rep cap documentation for nifgen

ce93c56

merge indices, string_indices fields

8e8833c

genereate correct rep cap documentation for niscope

cf13613

generate correct rep_cap documentation for niswitch

76fede8

parameterize the new tests

fb8c3a0

reduce code duplication in new helper functions

ad59a96

eliminate most rep cap example defaults

30efefd

ni-jfitzger requested a review from marcoskirsch July 11, 2023 20:18

ni-jfitzger commented Jul 11, 2023

View reviewed changes

docs/nidcpower/rep_caps.rst Outdated Show resolved Hide resolved

marcoskirsch requested changes Jul 12, 2023

View reviewed changes

ni-jfitzger added 3 commits July 15, 2023 12:21

Omit examples of elements and iterables at the top. Specify element t…

e461d65

…ype in documentation for each capability.

Remove the "or errors if the value is not the same for all." statement

19b2208

like -> such as

d2c61ae

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Repeated Capabilites documentation improvements #1686

Repeated Capabilites documentation improvements #1686

ni-jfitzger commented Jan 21, 2022 •

edited

Loading

ni-jfitzger Jan 21, 2022

nimi-bot commented Jan 21, 2022

ni-jfitzger commented Jan 21, 2022

marcoskirsch commented Jan 24, 2022

ni-jfitzger commented Jan 24, 2022

sbethur Jan 29, 2022

sbethur Jan 29, 2022

ni-jfitzger Jul 11, 2023

marcoskirsch Jul 12, 2023 •

edited

Loading

ni-jfitzger Jul 12, 2023

ni-jfitzger Jul 11, 2023

marcoskirsch Jul 12, 2023

marcoskirsch Jul 12, 2023

marcoskirsch Jul 12, 2023

marcoskirsch Jul 12, 2023

ni-jfitzger Jul 12, 2023

ni-jfitzger Jul 15, 2023

marcoskirsch Jul 17, 2023

marcoskirsch Jul 12, 2023

ni-jfitzger Jul 12, 2023 •

edited

Loading

marcoskirsch Jul 12, 2023

ni-jfitzger Jul 12, 2023

ni-jfitzger Jul 17, 2023


		Some repeated capabilities use a prefix before the number and this is optional
		The recommended way of accessing repeated capabilities is with an integer :python:`[0]` or range :python:`[0:2]`.


		Some repeated capabilities use a prefix before the number and this is optional
		The recommended way of accessing multiple repeated capabilites at once is with a tuple (:python:`[0, 1]` or :python:`['Dev1', 'Dev2']`) or slice :python:`[0:2]`.


		passes a string of :python:`'0, 1, 2'` to the set attribute function.
		session.channels[0, 2].output_function = nidcpower.OutputFunction.DC_CURRENT


		passes a string of :python:`'0, 1, 2'` to the set attribute function.
		print(session.instruments['Dev1', 'Dev2', '3rdDevice'].serial_number)

Repeated Capabilites documentation improvements #1686

Are you sure you want to change the base?

Repeated Capabilites documentation improvements #1686

Conversation

ni-jfitzger commented Jan 21, 2022 • edited Loading

What does this Pull Request accomplish?

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

What testing has been done?

Choose a reason for hiding this comment

nimi-bot commented Jan 21, 2022

ni-jfitzger commented Jan 21, 2022

marcoskirsch commented Jan 24, 2022

ni-jfitzger commented Jan 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcoskirsch Jul 12, 2023 • edited Loading

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

ni-jfitzger Jul 12, 2023 • edited Loading

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

ni-jfitzger commented Jan 21, 2022 •

edited

Loading

marcoskirsch Jul 12, 2023 •

edited

Loading

ni-jfitzger Jul 12, 2023 •

edited

Loading