# design Array fields for table version 1

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
6
• Sprint:
Measurement-S14-4
• Team:
Data Release Production

#### Description

While we're trying to eliminate the need for compound fields in afw::table, Arrays present a few problems. We could use FunctorKeys, the way we plan to use other compound fields, but here we need to guarantee that the per-element keys are contiguous, and we also be able to support views. We also need to determine the naming scheme. Finally, we need to make sure these work with aliases and slots.

#### Activity

Hide
Jim Bosch added a comment -

Ready for review. Perry, could you take a look?

Code is on u/jbosch/DM-836 of afw, and there's a GitHub Pull Request page you'll find linked on the right side of the issue page that shows the diff and all the commits involved, and gives the the ability to comment on individual lines in the diff (just click to the left of any line). Please make your comments there, and let me know if you have any questions about how the reviews on GitHub work.

Show
Jim Bosch added a comment - Ready for review. Perry, could you take a look? Code is on u/jbosch/ DM-836 of afw, and there's a GitHub Pull Request page you'll find linked on the right side of the issue page that shows the diff and all the commits involved, and gives the the ability to comment on individual lines in the diff (just click to the left of any line). Please make your comments there, and let me know if you have any questions about how the reviews on GitHub work.
Hide
Jim Bosch added a comment -

Taking this out of review temporarily; I've noticed a problem that I need to fix first.

Show
Jim Bosch added a comment - Taking this out of review temporarily; I've noticed a problem that I need to fix first.
Hide
Jim Bosch added a comment -

Fixed the bugs, so I'm putting it back in review. New commits should appear on the GitHub PR mentioned above.

Show
Jim Bosch added a comment - Fixed the bugs, so I'm putting it back in review. New commits should appear on the GitHub PR mentioned above.
Hide
Perry Gee added a comment -

I put my comments on GitHub. I should have listed linenos. and didn't, but let me know if it isn't obvious.

Show
Perry Gee added a comment - I put my comments on GitHub. I should have listed linenos. and didn't, but let me know if it isn't obvious.
Hide
Jim Bosch added a comment -

Summary of the review:

• Request for code comments in tests (done).
• Trivial doc fixes (done).
• Raised concern about behavior of BaseRecord.__getitem__ when passed an invalid Key; I looked closer at the code, and found that a lower-level call already included a check, so just letting that propagate up will do what we want.
• A question about the usefulness of the constructor that takes an ArrayKey; my reply is that it's useful in order to give the user more control over documentation and units for individual Keys in the array, and to provide a more efficient way to reconstruct an ArrayKey when we already have the Keys in hand.
Show
Jim Bosch added a comment - Summary of the review: Request for code comments in tests (done). Trivial doc fixes (done). Raised concern about behavior of BaseRecord.__getitem__ when passed an invalid Key; I looked closer at the code, and found that a lower-level call already included a check, so just letting that propagate up will do what we want. A question about the usefulness of the constructor that takes an ArrayKey ; my reply is that it's useful in order to give the user more control over documentation and units for individual Keys in the array, and to provide a more efficient way to reconstruct an ArrayKey when we already have the Keys in hand.
Hide
Robyn Allsman [X] (Inactive) added a comment -

I was just in the middle of reviewing it. I was still trying to get
through the code though, and hadn't yet noticed any problems.

Show
Robyn Allsman [X] (Inactive) added a comment - I was just in the middle of reviewing it. I was still trying to get through the code though, and hadn't yet noticed any problems.

#### People

Assignee:
Jim Bosch
Reporter:
Jim Bosch
Reviewers:
Perry Gee
Watchers:
Jim Bosch, Perry Gee, Robyn Allsman [X] (Inactive)