Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated page with allocation report #8558

Merged
merged 3 commits into from
Oct 21, 2024
Merged

Updated page with allocation report #8558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
32abd1f
updated allocation report
bsekachev Oct 17, 2024
8e47fe8
Missing line
bsekachev Oct 21, 2024
b598ff0
Refactoring
bsekachev Oct 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cvat-core/src/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import QualityReport from './quality-report';
import QualityConflict from './quality-conflict';
import QualitySettings from './quality-settings';
import AnalyticsReport from './analytics-report';
import ValidationLayout from './validation-layout';
import { JobValidationLayout, TaskValidationLayout } from './validation-layout';
import { Request } from './request';

import * as enums from './enums';
Expand Down Expand Up @@ -427,7 +427,8 @@ function build(): CVATCore {
QualityReport,
Request,
FramesMetaData,
ValidationLayout,
JobValidationLayout,
TaskValidationLayout,
},
utils: {
mask2Rle,
Expand Down
5 changes: 3 additions & 2 deletions cvat-core/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import QualityConflict from './quality-conflict';
import QualitySettings from './quality-settings';
import AnalyticsReport from './analytics-report';
import AnnotationGuide from './guide';
import ValidationLayout from './validation-layout';
import { JobValidationLayout, TaskValidationLayout } from './validation-layout';
import { Request } from './request';
import BaseSingleFrameAction, { listActions, registerAction, runActions } from './annotations-actions';
import {
Expand Down Expand Up @@ -216,7 +216,8 @@ export default interface CVATCore {
AnalyticsReport: typeof AnalyticsReport;
Request: typeof Request;
FramesMetaData: typeof FramesMetaData;
ValidationLayout: typeof ValidationLayout;
JobValidationLayout: typeof JobValidationLayout;
TaskValidationLayout: typeof TaskValidationLayout;
Comment on lines +219 to +220
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Issues Found: Residual ValidationLayout Usages Detected

The verification script identified remaining usages of the old ValidationLayout type in the following files:

  • cvat-ui/src/actions/annotation-actions.ts
  • cvat-core/src/actions/annotation-actions.ts

Please update these instances to use JobValidationLayout and TaskValidationLayout instead of the deprecated ValidationLayout to maintain consistency and ensure type safety throughout the codebase.

🔗 Analysis chain

LGTM: New validation layout types added to classes.

The addition of JobValidationLayout and TaskValidationLayout to the classes section is consistent with the import changes and represents a good improvement in type specificity. This change should enhance type safety and code clarity when working with validation layouts for jobs and tasks.

To ensure this change is consistently applied throughout the codebase, please run the following verification script:

Action Required: Update verification scripts due to errors in initial execution.

Action Required: Correct the verification scripts to address ripgrep errors.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining usage of the old ValidationLayout type and verify the usage of new types.

# Test 1: Check for any remaining usage of ValidationLayout
echo "Checking for any remaining usage of ValidationLayout:"
rg --type typescript "ValidationLayout(?!.*JobValidationLayout.*TaskValidationLayout)" -g '!cvat-core/src/index.ts'

# Test 2: Verify the usage of new JobValidationLayout and TaskValidationLayout types
echo "Verifying usage of new JobValidationLayout and TaskValidationLayout types:"
rg --type typescript "JobValidationLayout|TaskValidationLayout"

# Note: Review the output to ensure that ValidationLayout has been replaced everywhere necessary,
# and that JobValidationLayout and TaskValidationLayout are being used appropriately.

Length of output: 777

Script:

#!/bin/bash
# Description: Check for any remaining usage of the old ValidationLayout type and verify the usage of new types.

# Test 1: Check for any remaining usage of ValidationLayout using PCRE2 to support look-around
echo "Checking for any remaining usage of ValidationLayout:"
rg --pcre2 --type typescript "ValidationLayout(?!.*JobValidationLayout.*TaskValidationLayout)" -g '!cvat-core/src/index.ts'

# Test 2: Verify the usage of new JobValidationLayout and TaskValidationLayout types using file extensions
echo "Verifying usage of new JobValidationLayout and TaskValidationLayout types:"
rg "JobValidationLayout|TaskValidationLayout" -g '*.ts' -g '*.tsx'

# Note: Review the output to ensure that ValidationLayout has been replaced everywhere necessary,
# and that JobValidationLayout and TaskValidationLayout are being used appropriately.

Length of output: 5658

Script:

#!/bin/bash
# Description: Check for any remaining usage of the old ValidationLayout type and verify the usage of new types.

# Test 1: Check for any remaining usage of ValidationLayout using PCRE2 to support look-around
echo "Checking for any remaining usage of ValidationLayout:"
rg --pcre2 "ValidationLayout(?!.*JobValidationLayout.*TaskValidationLayout)" -g '*.ts' -g '*.tsx' -g '!cvat-core/src/index.ts'

# Test 2: Verify the usage of new JobValidationLayout and TaskValidationLayout types using file extensions
echo "Verifying usage of new JobValidationLayout and TaskValidationLayout types:"
rg "JobValidationLayout|TaskValidationLayout" -g '*.ts' -g '*.tsx'

# Note: Review the output to ensure that ValidationLayout has been replaced everywhere necessary,
# and that JobValidationLayout and TaskValidationLayout are being used appropriately.

Length of output: 10973

};
utils: {
mask2Rle: typeof mask2Rle;
Expand Down
4 changes: 2 additions & 2 deletions cvat-core/src/server-proxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
SerializedInvitationData, SerializedCloudStorage, SerializedFramesMetaData, SerializedCollection,
SerializedQualitySettingsData, APIQualitySettingsFilter, SerializedQualityConflictData, APIQualityConflictsFilter,
SerializedQualityReportData, APIQualityReportsFilter, SerializedAnalyticsReport, APIAnalyticsReportFilter,
SerializedRequest, SerializedValidationLayout,
SerializedRequest, SerializedJobValidationLayout, SerializedTaskValidationLayout,
} from './server-response-types';
import { PaginatedResource } from './core-types';
import { Request } from './request';
Expand Down Expand Up @@ -244,7 +244,7 @@
return new ServerError(message, 0);
}

function prepareData(details) {

Check warning on line 247 in cvat-core/src/server-proxy.ts

View workflow job for this annotation

GitHub Actions / Linter 

Missing return type on function
const data = new FormData();
for (const [key, value] of Object.entries(details)) {
if (Array.isArray(value)) {
Expand Down Expand Up @@ -286,7 +286,7 @@
return requestId++;
}

async function get(url: string, requestConfig) {

Check warning on line 289 in cvat-core/src/server-proxy.ts

View workflow job for this annotation

GitHub Actions / Linter 

Missing return type on function
return new Promise((resolve, reject) => {
const newRequestId = getRequestId();
requests[newRequestId] = { resolve, reject };
Expand Down Expand Up @@ -819,7 +819,7 @@
save_images: saveImages,
};
return new Promise<string | void>((resolve, reject) => {
async function request() {

Check warning on line 822 in cvat-core/src/server-proxy.ts

View workflow job for this annotation

GitHub Actions / Linter 

Missing return type on function
Axios.post(baseURL, {}, {
params,
})
Expand Down Expand Up @@ -914,7 +914,7 @@
const url = `${backendAPI}/tasks/${id}/backup/export`;

return new Promise<string | void>((resolve, reject) => {
async function request() {

Check warning on line 917 in cvat-core/src/server-proxy.ts

View workflow job for this annotation

GitHub Actions / Linter 

Missing return type on function
try {
const response = await Axios.post(url, {}, {
params,
Expand Down Expand Up @@ -996,7 +996,7 @@
const url = `${backendAPI}/projects/${id}/backup/export`;

return new Promise<string | void>((resolve, reject) => {
async function request() {

Check warning on line 999 in cvat-core/src/server-proxy.ts

View workflow job for this annotation

GitHub Actions / Linter 

Missing return type on function
try {
const response = await Axios.post(url, {}, {
params,
Expand Down Expand Up @@ -1124,7 +1124,7 @@
message: 'CVAT is uploading task data to the server',
}));

async function bulkUpload(taskId, files) {

Check warning on line 1127 in cvat-core/src/server-proxy.ts

View workflow job for this annotation

GitHub Actions / Linter 

Missing return type on function
const fileBulks = files.reduce((fileGroups, file) => {
const lastBulk = fileGroups[fileGroups.length - 1];
if (chunkSize - lastBulk.size >= file.size) {
Expand Down Expand Up @@ -1384,7 +1384,7 @@

const validationLayout = (instance: 'tasks' | 'jobs') => async (
id: number,
): Promise<SerializedValidationLayout | null> => {
): Promise<SerializedJobValidationLayout | SerializedTaskValidationLayout> => {
const { backendAPI } = config;

try {
Expand Down
8 changes: 7 additions & 1 deletion cvat-core/src/server-response-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,8 +524,14 @@ export interface SerializedRequest {
owner?: any;
}

export interface SerializedValidationLayout {
export interface SerializedJobValidationLayout {
honeypot_count?: number;
honeypot_frames?: number[];
honeypot_real_frames?: number[];
}

export interface SerializedTaskValidationLayout extends SerializedJobValidationLayout {
mode: 'gt' | 'gt_pool' | null;
validation_frames?: number[];
disabled_frames?: number[];
}
15 changes: 9 additions & 6 deletions cvat-core/src/session-implementation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ import {
decodePreview,
} from './frames';
import Issue from './issue';
import { SerializedLabel, SerializedTask, SerializedValidationLayout } from './server-response-types';
import {
SerializedLabel, SerializedTask, SerializedJobValidationLayout,
SerializedTaskValidationLayout,
} from './server-response-types';
import { checkInEnum, checkObjectType } from './common';
import {
getCollection, getSaver, clearAnnotations, getAnnotations,
Expand All @@ -37,7 +40,7 @@ import AnnotationGuide from './guide';
import requestsManager from './requests-manager';
import { Request } from './request';
import User from './user';
import ValidationLayout from './validation-layout';
import { JobValidationLayout, TaskValidationLayout } from './validation-layout';

// must be called with task/job context
async function deleteFrameWrapper(jobID, frame): Promise<void> {
Expand Down Expand Up @@ -171,7 +174,7 @@ export function implementJob(Job: typeof JobClass): typeof JobClass {
): ReturnType<typeof JobClass.prototype.validationLayout> {
const result = await serverProxy.jobs.validationLayout(this.id);
if (Object.keys(result).length) {
return new ValidationLayout(result as Required<SerializedValidationLayout>);
return new JobValidationLayout(result as SerializedJobValidationLayout);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure 'result' is not null or undefined before accessing its properties

In the Job.prototype.validationLayout implementation, there is a check for Object.keys(result).length, but if result is null or undefined, accessing Object.keys(result) can cause a runtime error.

Consider adding a null check for result before accessing its properties:

 async function validationLayoutImplementation(
     this: JobClass,
 ): ReturnType<typeof JobClass.prototype.validationLayout> {
     const result = await serverProxy.jobs.validationLayout(this.id);
-    if (Object.keys(result).length) {
+    if (result && Object.keys(result).length) {
         return new JobValidationLayout(result as SerializedJobValidationLayout);
     }
     return null;
 }

Committable suggestion was skipped due to low confidence.

}

return null;
Expand Down Expand Up @@ -641,9 +644,9 @@ export function implementTask(Task: typeof TaskClass): typeof TaskClass {
value: async function validationLayoutImplementation(
this: TaskClass,
): ReturnType<typeof TaskClass.prototype.validationLayout> {
const result = await serverProxy.tasks.validationLayout(this.id);
if (Object.keys(result).length) {
return new ValidationLayout(result as Required<SerializedValidationLayout>);
const result = await serverProxy.tasks.validationLayout(this.id) as SerializedTaskValidationLayout;
if (result.mode !== null) {
return new TaskValidationLayout(result);
Comment on lines +647 to +649
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add null check for 'result' before accessing 'result.mode'

In the Task.prototype.validationLayout implementation, there is an assumption that result is not null or undefined. If serverProxy.tasks.validationLayout(this.id) returns null or undefined, accessing result.mode will result in a runtime error.

Recommend adding a null check for result:

 const result = await serverProxy.tasks.validationLayout(this.id) as SerializedTaskValidationLayout;
- if (result.mode !== null) {
+ if (result && result.mode !== null) {
     return new TaskValidationLayout(result);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const result = await serverProxy.tasks.validationLayout(this.id) as SerializedTaskValidationLayout;
if (result.mode !== null) {
return new TaskValidationLayout(result);
const result = await serverProxy.tasks.validationLayout(this.id) as SerializedTaskValidationLayout;
if (result && result.mode !== null) {
return new TaskValidationLayout(result);

}

return null;
Expand Down
6 changes: 3 additions & 3 deletions cvat-core/src/session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { Request } from './request';
import logger from './logger';
import Issue from './issue';
import ObjectState from './object-state';
import ValidationLayout from './validation-layout';
import { JobValidationLayout, TaskValidationLayout } from './validation-layout';

function buildDuplicatedAPI(prototype) {
Object.defineProperties(prototype, {
Expand Down Expand Up @@ -686,7 +686,7 @@ export class Job extends Session {
return result;
}

async validationLayout(): Promise<ValidationLayout | null> {
async validationLayout(): Promise<JobValidationLayout | null> {
const result = await PluginRegistry.apiWrapper.call(this, Job.prototype.validationLayout);
return result;
}
Expand Down Expand Up @@ -1186,7 +1186,7 @@ export class Task extends Session {
return result;
}

async validationLayout(): Promise<ValidationLayout | null> {
async validationLayout(): Promise<TaskValidationLayout | null> {
const result = await PluginRegistry.apiWrapper.call(this, Task.prototype.validationLayout);
return result;
}
Expand Down
55 changes: 43 additions & 12 deletions cvat-core/src/validation-layout.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,43 @@
//
// SPDX-License-Identifier: MIT

import { SerializedValidationLayout } from 'server-response-types';
import { SerializedJobValidationLayout, SerializedTaskValidationLayout } from 'server-response-types';
import PluginRegistry from './plugins';

export default class ValidationLayout {
#honeypotFrames: number[];
#honeypotRealFrames: number[];
export class JobValidationLayout {
#honeypotCount: JobValidationLayout['honeypotCount'];
#honeypotFrames: JobValidationLayout['honeypotFrames'];
#honeypotRealFrames: JobValidationLayout['honeypotRealFrames'];
Comment on lines +8 to +11
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid circular type references in private field declarations

Defining private fields with types that reference themselves can lead to circular type references, which might cause issues with type checking and readability.

Consider defining the types explicitly:

-export class JobValidationLayout {
-    #honeypotCount: JobValidationLayout['honeypotCount'];
-    #honeypotFrames: JobValidationLayout['honeypotFrames'];
-    #honeypotRealFrames: JobValidationLayout['honeypotRealFrames'];
+export class JobValidationLayout {
+    #honeypotCount: number;
+    #honeypotFrames: number[];
+    #honeypotRealFrames: number[];
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export class JobValidationLayout {
#honeypotCount: JobValidationLayout['honeypotCount'];
#honeypotFrames: JobValidationLayout['honeypotFrames'];
#honeypotRealFrames: JobValidationLayout['honeypotRealFrames'];
export class JobValidationLayout {
#honeypotCount: number;
#honeypotFrames: number[];
#honeypotRealFrames: number[];


public constructor(data: Required<SerializedValidationLayout>) {
this.#honeypotFrames = [...data.honeypot_frames];
this.#honeypotRealFrames = [...data.honeypot_real_frames];
public constructor(data: SerializedJobValidationLayout) {
this.#honeypotCount = data.honeypot_count ?? 0;
this.#honeypotFrames = [...(data.honeypot_frames ?? [])];
this.#honeypotRealFrames = [...(data.honeypot_real_frames ?? [])];
}

public get honeypotFrames() {
public get honeypotCount(): number {
return this.#honeypotCount;
}

public get honeypotFrames(): number[] {
return [...this.#honeypotFrames];
}

public get honeypotRealFrames() {
public get honeypotRealFrames(): number[] {
return [...this.#honeypotRealFrames];
}

async getRealFrame(frame: number): Promise<number | null> {
const result = await PluginRegistry.apiWrapper.call(this, ValidationLayout.prototype.getRealFrame, frame);
const result = await PluginRegistry.apiWrapper.call(this, JobValidationLayout.prototype.getRealFrame, frame);
return result;
}
}

Object.defineProperties(ValidationLayout.prototype.getRealFrame, {
Object.defineProperties(JobValidationLayout.prototype.getRealFrame, {
implementation: {
writable: false,
enumerable: false,
value: function implementation(this: ValidationLayout, frame: number): number | null {
value: function implementation(this: JobValidationLayout, frame: number): number | null {
const index = this.honeypotFrames.indexOf(frame);
if (index !== -1) {
return this.honeypotRealFrames[index];
Expand All @@ -42,3 +48,28 @@ Object.defineProperties(ValidationLayout.prototype.getRealFrame, {
},
},
});

export class TaskValidationLayout extends JobValidationLayout {
#mode: TaskValidationLayout['mode'];
#validationFrames: TaskValidationLayout['validationFrames'];
#disabledFrames: TaskValidationLayout['disabledFrames'];
Comment on lines +52 to +55
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid circular type references in TaskValidationLayout's private fields

Similar to JobValidationLayout, the private fields in TaskValidationLayout are defined using types that reference themselves, which can cause circular references.

Consider declaring the types explicitly:

export class TaskValidationLayout extends JobValidationLayout {
-    #mode: TaskValidationLayout['mode'];
-    #validationFrames: TaskValidationLayout['validationFrames'];
-    #disabledFrames: TaskValidationLayout['disabledFrames'];
+    #mode: NonNullable<SerializedTaskValidationLayout['mode']>;
+    #validationFrames: number[];
+    #disabledFrames: number[];
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export class TaskValidationLayout extends JobValidationLayout {
#mode: TaskValidationLayout['mode'];
#validationFrames: TaskValidationLayout['validationFrames'];
#disabledFrames: TaskValidationLayout['disabledFrames'];
export class TaskValidationLayout extends JobValidationLayout {
#mode: NonNullable<SerializedTaskValidationLayout['mode']>;
#validationFrames: number[];
#disabledFrames: number[];


public constructor(data: SerializedTaskValidationLayout) {
super(data);
this.#mode = data.mode;
this.#validationFrames = [...(data.validation_frames ?? [])];
this.#disabledFrames = [...(data.disabled_frames ?? [])];
}

public get mode(): NonNullable<SerializedTaskValidationLayout['mode']> {
return this.#mode;
}

public get validationFrames(): number[] {
return [...this.#validationFrames];
}

public get disabledFrames(): number[] {
return [...this.#disabledFrames];
}
}
4 changes: 2 additions & 2 deletions cvat-ui/src/actions/annotation-actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
} from 'cvat-canvas-wrapper';
import {
getCore, MLModel, JobType, Job, QualityConflict,
ObjectState, JobState, ValidationLayout,
ObjectState, JobState, JobValidationLayout,
} from 'cvat-core-wrapper';
import logger, { EventScope } from 'cvat-logger';
import { getCVATStore } from 'cvat-store';
Expand All @@ -38,7 +38,7 @@ interface AnnotationsParameters {
showGroundTruth: boolean;
jobInstance: Job;
groundTruthInstance: Job | null;
validationLayout: ValidationLayout | null;
validationLayout: JobValidationLayout | null;
}

const cvat = getCore();
Expand Down
Loading
Loading