Desktop Attributes by BryonLewis · Pull Request #546 · Kitware/dive

BryonLewis · 2021-01-26T19:12:58Z

Adding in the capability to load/create and edit track attributes within the desktop version.
TestData: https://viame.kitware.com/#/viewer/6006e01cf751cc24d135ec2b - has attributes on track #1 I believe. Remember that on web version you won't see attributes unless they are added to the global list of attributes. This isn't required for the desktop version.

Explanation:

Process CSV import data to determine track attributes. Currently done outside of the actual CSV importation which isn't the most performant but is organized a bit better. I essentially took advantage of the outside loop used to process the TrackData[] into a MultiTrackRecord. Let me know if you feel strongly about this being injected directly into the parser. I like how the parser returns a clean Array right now.
The AttributeProcessor is a function used to iterate through the TrackData[] and gather up the used attributes as well as their values and number of times those values occur. It does a secondary pass of the attributes to try to determine the attribute type. It cascades from number to boolean to text based on attempting to covert all of the possible set values for that attribute. Finally it counts the number of occurrences of the value and if it is text and all values are used at least twice it will infer that it is a set of discrete values.
When we have the ability to load the meta.json that will override any settings that are calculated with the csv/json file itself.
Modified the API for attributes to include the datasetId. This is necessary for desktop to access the proper metadata file to update it when attributes are edited. Note these are typically one time things that occur when adding/editing/deleting an attribute from the system or on mount for the TrackDetailsView. The web version ignores this data, but in the future will be used to directly modify the metadata attributes field utilizing the same endpoints instead of storing it globally in girder.

Questions:

Should parsing for attributes be moved into CSV Parser or left with the TrackData[] -> MultiTrackRecord conversion?
Should I remove the inferring of type and leave all items as text?
Should the attributeProcessor.ts be it's own thing or moved into utils.ts?

TODO (Future PR):

Setting/Updating/Deleting attributes both for the web version and the desktop version affect the displaying of attributes in the editor but not the source data of the track JSON. Technically deleting an attribute should search all of the tracks and remove the attribute wherever it is set in the JSON field. I'm wondering if that should be implemented. This is an expensive task based on the number of detections and attributes in the system. May want to have a discussion on how to implement this.

subdavis

Remove comments and console.log

Possibly remove loadAttributes and its API call.

subdavis · 2021-01-28T18:41:38Z

            });
          } else {
            // Display new data and await transcoding to complete
+            console.log(meta);


subdavis · 2021-01-28T18:44:47Z

    >
      <sidebar
-        v-bind="{ newTrackSettings, typeSettings }"
+        v-bind="{ newTrackSettings, typeSettings, datasetId:id }"


It's possible that provides.State would have been a better place to put this, especially considering everything in state belongs to exactly one dataset. I'm not going to dwell on this, it's good like it is for now.

My brain was broken at the moment. I knew internally that this is something that would be in a global store in vuex terms and the provides stuff just slipped my mind for some reason. I'll look into modifying it and providing it from there.

subdavis · 2021-01-28T18:46:43Z

@@ -112,19 +112,25 @@ async function saveDetections(id: string, args: SaveDetectionsArgs) {
 /**
 * Unimplemented sections of the API


This comment is incorrect now.

subdavis · 2021-01-28T18:46:53Z

+  return client.post(`dataset/${datasetId}/attribute`, { addNew, data });
 }

 // eslint-disable-next-line @typescript-eslint/no-unused-vars


these comments should be removed.

subdavis · 2021-01-28T18:58:35Z

Should parsing for attributes be moved into CSV Parser or left with the TrackData[] -> MultiTrackRecord conversion?

I like it how it is.

Should I remove the inferring of type and leave all items as text?

Type inference seems handy, but if it turns out to be unreliable, we can remove it.

Should the attributeProcessor.ts be it's own thing or moved into utils.ts?

I like it in its own file.

BryonLewis · 2021-01-29T17:32:28Z

Did the comment updates and I created a useDatasetId to provide the datasetID to the track-details. This is much better.

subdavis · 2021-01-29T18:18:36Z

Something weird is going on.

Track and detection attributes seem to erase each other.

weird.movie.mp4

BryonLewis · 2021-01-29T18:38:04Z

Something weird is going on.

Track and detection attributes seem to erase each other.

Wasn't thinking right. On girder the _id is provided for you when you create a new attribute hence you could get conflicts with the same name. So it wasn't setting the _id properly.

… into desktop/attributes

subdavis

Looks great.

One last thing: can we change the default visibility mode to always show the editable fields? Its fine that you can have the nice condensed view, but if you create a new attribute, it doesn't show up until you hit the eye, which I don't think it a good default.

This optimization should be opt-in rather than opt-out.

BryonLewis · 2021-02-01T15:06:56Z

Looks great.

One last thing: can we change the default visibility mode to always show the editable fields? Its fine that you can have the nice condensed view, but if you create a new attribute, it doesn't show up until you hit the eye, which I don't think it a good default.

This optimization should be opt-in rather than opt-out.

I agree that after adding a new attribute it should be immediately available. I think we can change it so that if you add a new attribute it will swap to show all the fields after adding it. The main concern I have is that there is a difference between viewing attributes and actively editing them. Some of my test datasets have 10-15 track and then another 10-15 detection attributes. They would all be displayed in the list by default meaning that it's difficult to view the data because of the sparseness especially if flipping through tracks or frames. I'm open to any other alternatives if you think of any, maybe the eye should be something else?

subdavis · 2021-02-01T15:17:51Z

I'm just sort of looking at the risks:

"too busy, hard to parse, I'm overwhelmed" vs "I think this is broken it's not working I'm frustrated"

seems like we shoud choose the first risk. It's a hard problem and I can't come up with a solution that will work for everyone.

subdavis · 2021-02-01T15:19:19Z

There's always the "show more" / "show less" option at the end of the list. Those are pretty ubiquitous, might be more intuitive than the eye, but I still think "show more" is a better default.

BryonLewis and others added 6 commits January 26, 2021 13:57

support for adding/editing desktop attributes

577cbb6

Merge branch 'master' into desktop/attributes

e73fae3

adding in track.json support for attributes

ece2bf5

refactor attributeProcess

5243a4f

small structure and comment changes

444cd71

Merge branch 'master' into desktop/attributes

2b1ea86

BryonLewis marked this pull request as ready for review January 28, 2021 13:21

BryonLewis requested a review from subdavis January 28, 2021 13:31

subdavis suggested changes Jan 28, 2021

View reviewed changes

BryonLewis and others added 2 commits January 29, 2021 11:07

addressing comments

019b8e7

Merge branch 'master' into desktop/attributes

2456b25

BryonLewis requested a review from subdavis January 29, 2021 17:32

BryonLewis added 3 commits January 29, 2021 13:42

fixing attribute setting error

09d21b0

Merge branch 'desktop/attributes' of https://github.com/VIAME/VIAME-Web…

9a6b50a

… into desktop/attributes

Adding in some tests for attributes

c9629c1

subdavis suggested changes Feb 1, 2021

View reviewed changes

enabling all attributes view by default

6b39a6a

subdavis self-requested a review February 1, 2021 15:31

subdavis approved these changes Feb 1, 2021

View reviewed changes

BryonLewis merged commit 2eadde4 into master Feb 1, 2021

BryonLewis deleted the desktop/attributes branch February 3, 2021 17:31

		@@ -112,19 +112,25 @@ async function saveDetections(id: string, args: SaveDetectionsArgs) {
		/**
		* Unimplemented sections of the API

Conversation

BryonLewis commented Jan 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

subdavis left a comment

Choose a reason for hiding this comment

Uh oh!

subdavis Jan 28, 2021

Choose a reason for hiding this comment

Uh oh!

subdavis Jan 28, 2021

Choose a reason for hiding this comment

Uh oh!

BryonLewis Jan 28, 2021

Choose a reason for hiding this comment

Uh oh!

subdavis Jan 28, 2021

Choose a reason for hiding this comment

Uh oh!

subdavis Jan 28, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

subdavis commented Jan 28, 2021

Uh oh!

BryonLewis commented Jan 29, 2021

Uh oh!

subdavis commented Jan 29, 2021

Uh oh!

BryonLewis commented Jan 29, 2021

Uh oh!

subdavis left a comment

Choose a reason for hiding this comment

Uh oh!

BryonLewis commented Feb 1, 2021

Uh oh!

subdavis commented Feb 1, 2021

Uh oh!

subdavis commented Feb 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

BryonLewis commented Jan 26, 2021 •

edited

Loading