Skip to content

Fixing negative Coordinates in polygons #1145

Merged
BryonLewis merged 10 commits into
mainfrom
fixes-993
Feb 7, 2022
Merged

Fixing negative Coordinates in polygons #1145
BryonLewis merged 10 commits into
mainfrom
fixes-993

Conversation

@BryonLewis
Copy link
Copy Markdown
Collaborator

@BryonLewis BryonLewis commented Feb 1, 2022

fixes #993
fixes #1152

  • Simple fix involved adding optional - specifiers to the RegEx for polygon. Decided it should be added to head tail points as well and did it in both desktop and python.
  • Updated the unit tests to account for negative values. Doing that I also decided that I wanted unified tests between python and node so I refactored the viame serializers to use the same JSON file.
  • Noticed there were discrepancies in the outputs of Python vs Node so I remedied them:
    • fishLength was always set to -1 on Node but set to None in Python so I made it so both will set it to None/undefined if it equals -1
    • Node gave each track a meta:{} attribute that was never used. Python didn't read in or create this attribute so I removed the automatic creation from the node version. I added the meta: Optional[Dict[str, Any]] to make sure that the type specifications between the two were the same.
    • Python had each detections start with the properties interpolate:false and keyframe:true. I added both of these to the Node version so there is parity.

1152 Fix (attributes were not loading properly)

  • The single change in common.ts - meta.attributes = processed.attributes; should fix this. It wasn't loading the attributes into the metafile so they weren't being populated in the interface.
  • I also updated the common.spec.ts to do a sort of end to end loading of folder and check the attributes. This is like above where I use the viame.spec.json to do this.

@BryonLewis BryonLewis marked this pull request as ready for review February 3, 2022 15:03
@subdavis subdavis mentioned this pull request Feb 7, 2022
Copy link
Copy Markdown
Contributor

@subdavis subdavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple comments, and also see #1155

Comment on lines +112 to +129
// Adding in some ingest pairs
type testPairs = [string[], MultiTrackRecord, Record<string, Attribute>];

const testData: testPairs[] = fs.readJSONSync('../testutils/viame.spec.json');
const images: Record<string, string> = {};
const imageList = Array.from(Array(10).keys());
imageList.shift(); //remove 0.png
// eslint-disable-next-line no-return-assign
imageList.forEach((item) => images[`${item}.png`] = '');

type TestKey = string | 'annotations.csv';
const fileSystemData: Record<string, Record<string, string>> = { };
testData.forEach((triplet, index) => {
fileSystemData[`test${index}`] = {
...images,
'annotations.csv': triplet[0].join('\n'),
};
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section is a little hard to read. You seem to be preparing some data strictures to be used by tests, but I don't know where this information is coming from.

  • What is imageList? Where did that come from?
  • What is going on with shift()
  • In general, what is being prepared in this section, and what does it have to match up with?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this because while we had the viame serializer test with the attributes that was passing. We also had an invidiual test of attributes in attributes.spec.ts that was passing. We didn't have a test that made sure these were hooked up together and working properly. This is how we were getting pipelines that could run without having the attributes show up in the meta.json. So this is running a fuller test from loading the images/annotations.csv from a mockfs directory so it ensures that viame serializer and the attributes processor are working together and producing the desired meta.json outcome. Hopefully my newer comments cleared it up a bit.

Comment thread client/platform/desktop/backend/serializers/viame.spec.ts
@BryonLewis BryonLewis merged commit 2a00bca into main Feb 7, 2022
@BryonLewis BryonLewis deleted the fixes-993 branch February 7, 2022 21:37
BryonLewis added a commit that referenced this pull request Feb 24, 2022
* fixes-993

* Adding tests and other regexes

* unifying testing

* adding in meta field to python

* Adding in meta attributes test

* fix image name for tests

* fixing ordering of viame.spec.json

* No keyframe or interpolate defaults (#1155)

* adding clarification/documentation to unit test

Co-authored-by: Brandon Davis <git@subdavis.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants