Make sites into repeated capabilities #1290

texasaggie97-zz · 2020-02-24T21:22:21Z

This contribution adheres to CONTRIBUTING.md.
I've updated CHANGELOG.md if applicable.
I've ~~added~~updated tests applicable for this pull request

What does this Pull Request accomplish?

Moves sites from parameter to repeated capbility

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

Fix nidigital should expose sites as repeated capabilities #1111

What testing has been done?

System

…, we need to explictly say these are

codecov · 2020-02-24T21:26:16Z

Codecov Report

Merging #1290 into master will decrease coverage by 4.25%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1290      +/-   ##
==========================================
- Coverage   96.09%   91.84%   -4.26%     
==========================================
  Files          10       20      +10     
  Lines        1665     3616    +1951     
==========================================
+ Hits         1600     3321    +1721     
- Misses         65      295     +230

Flag	Coverage Δ
#codegenunittests	`88.21% <ø> (?)`
#nifakeunittests	`96.3% <ø> (ø)`	⬆️
#nimodinstunittests	`96.19% <ø> (ø)`	⬆️
#nitclkunittests	`96.09% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
generated/nifake/nifake/_converters.py	`96.42% <ø> (ø)`	⬆️
build/helper/documentation_helper.py	`89.42% <0%> (ø)`
build/helper/parameter_usage_options.py	`100% <0%> (ø)`
build/helper/helper.py	`88.28% <0%> (ø)`
build/helper/documentation_snippets.py	`90.8% <0%> (ø)`
build/helper/__init__.py	`100% <0%> (ø)`
build/helper/metadata_add_all.py	`81.35% <0%> (ø)`
build/helper/metadata_merge_dicts.py	`89.01% <0%> (ø)`
build/helper/metadata_filters.py	`78.16% <0%> (ø)`
build/helper/codegen_helper.py	`92.35% <0%> (ø)`
... and 1 more

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 a8b5cb6...256b389. Read the comment docs.

generated/nidigital/nidigital/session.py

src/nidigital/system_tests/test_system_nidigital.py

src/nidigital/metadata/functions.py

Comments addresssed

src/nidigital/metadata/functions.py

build/templates/_converters.py.mako

src/nidigital/metadata/functions.py

src/nidigital/system_tests/test_system_nidigital.py

sbethur

Also, please merge latest from master.

…ually get ran

Comments addressed

src/nidigital/metadata/functions.py

build/templates/_converters.py.mako

generated/nidmm/nidmm/_converters.py

build/templates/_converters.py.mako

docs/nidigital/rep_caps.rst

src/nidigital/system_tests/test_system_nidigital.py

src/nidigital/templates/session.py/fancy_fetch_history_ram_cycle_information.py.mako

Comments addressed

sbethur

~~A new fancy function got added in another PR. You need to add sites rep-cap to that function and update the system test.~~

It has already been taken care of.

marcoskirsch

I have questions on why some methods have a site parameter rather than use the repeated capabilities. I suspect it is because only one site is accepted. But I don't think that's a convincing argument. We have methods that use the channels repeated capability in other APIs and only allow specifying one at a time.

We should discuss this in the API review.

marcoskirsch · 2020-03-20T01:53:19Z

docs/nidigital/class.rst



-            :type site: str
+            :type site: str or int


why didn't we make the site parameter a repeated capability?

marcoskirsch · 2020-03-20T01:53:47Z

docs/nidigital/class.rst



-            :type site: str
+            :type site: str or int


why didn't we make the site parameter a repeated capability?

marcoskirsch · 2020-03-20T01:54:26Z

docs/nidigital/class.rst



-            :type site: str
+            :type site: str or int


why didn't we make the site parameter a repeated capability?

texasaggie97-zz added 5 commits February 24, 2020 15:10

Add sites as a repeated capability

eda7e5e

Since sites are not one of the "magic" terms for repeated capabilites…

804b90d

…, we need to explictly say these are

Update generated files

e94ecfa

Update CHANGELOG.md

b194905

Missed fancy fetch capture waveform

405158c

texasaggie97-zz added 3 commits February 24, 2020 15:53

write_source_waveform_site_unique also uses site

cb94b7c

site_list is a rep cap now so remove from parameter list

a80f88a

site_list is not a param

12c65c5

sbethur previously requested changes Feb 28, 2020

View reviewed changes

generated/nidigital/nidigital/session.py Outdated Show resolved Hide resolved

src/nidigital/system_tests/test_system_nidigital.py Outdated Show resolved Hide resolved

sbethur reviewed Feb 28, 2020

View reviewed changes

src/nidigital/metadata/functions.py Show resolved Hide resolved

marcoskirsch mentioned this pull request Feb 28, 2020

Add fancy History RAM API method to allow fetching multiple History RAM samples in a single API call #1274

Merged

3 tasks

texasaggie97-zz added 2 commits March 2, 2020 17:59

Update for review comments

ef15749

Merge branch 'master' into bug1111/sites_rep_cap

f9d13b6

sbethur reviewed Mar 4, 2020

View reviewed changes

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

texasaggie97-zz added 8 commits March 4, 2020 13:04

Merge branch 'master' into bug1111/sites_rep_cap

1b6159b

Add nidigital specific converter for site (singular)

7dfe84d

Use site converter

52e9ad8

Add unit tests

9233c89

Update generated files

99a86af

Merge branch 'master' into bug1111/sites_rep_cap

025bbab

Update generated files

66bf427

Update test_system_nidigital.py

0d19816

sbethur previously requested changes Mar 10, 2020

View reviewed changes

sbethur reviewed Mar 10, 2020

View reviewed changes

texasaggie97-zz added 4 commits March 11, 2020 14:12

Change comments per review

9435805

Move nidigital converter tests to nidigital system tests, so they act…

b20b59b

…ually get ran

Fix documentation type

798b8ce

Update fetch_history_ram_cycle_information to properly support int sites

756bafc

texasaggie97-zz added 5 commits March 11, 2020 14:42

Wrong function name

6d3d503

Merge branch 'master' into bug1111/sites_rep_cap

e3ad36b

Update generated files

dff8c94

Remove blank line

258390d

Only use converter in the public method

3a668f3

sbethur approved these changes Mar 12, 2020

View reviewed changes

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

texasaggie97-zz added 3 commits March 12, 2020 10:58

Update type_in_documentation

0527b3d

Merge branch 'master' into bug1111/sites_rep_cap

2963ccc

Update generated files

3ad86f2

marcoskirsch previously requested changes Mar 18, 2020

View reviewed changes

sbethur mentioned this pull request Mar 18, 2020

Improve usability of get_site_pass_fail by returning dict - site: pass_fail #1329

Merged

3 tasks

texasaggie97-zz added 2 commits March 19, 2020 10:55

Merge branch 'master' into bug1111/sites_rep_cap

f709769

Update generated files

0913c05

texasaggie97-zz mentioned this pull request Mar 19, 2020

Poorly worded repeated capabilities documentation #1334

Open

texasaggie97-zz added 2 commits March 19, 2020 11:12

Updates for review comments

e4c65ed

Update generated files

46533cb

texasaggie97-zz added 6 commits March 19, 2020 14:17

Update system tests

12ee1dc

Merge branch 'master' into bug1111/sites_rep_cap

fc285c0

Remove site_list from fancy function

9ec17d9

Updates for tests

8aa00fa

Rely on repeated capability

e50b31e

Update generated files

cfa40ad

sbethur reviewed Mar 19, 2020

View reviewed changes

Make converter name correct

256b389

marcoskirsch approved these changes Mar 20, 2020

View reviewed changes

marcoskirsch mentioned this pull request Mar 20, 2020

Some methods in nidigital could (should?) use sites rep cap #1337

Closed

marcoskirsch merged commit b93dbc3 into master Mar 20, 2020

marcoskirsch deleted the bug1111/sites_rep_cap branch March 20, 2020 02:01

Make sites into repeated capabilities #1290

Make sites into repeated capabilities #1290

Uh oh!

Conversation

texasaggie97-zz commented Feb 24, 2020

What does this Pull Request accomplish?

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

What testing has been done?

Uh oh!

codecov bot commented Feb 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sbethur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sbethur left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoskirsch left a comment

Choose a reason for hiding this comment

Uh oh!

marcoskirsch Mar 20, 2020

Choose a reason for hiding this comment

Uh oh!

marcoskirsch Mar 20, 2020

Choose a reason for hiding this comment

Uh oh!

marcoskirsch Mar 20, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Feb 24, 2020 •

edited

Loading

sbethur left a comment •

edited

Loading