From 287d966e12ef1bb1d852f6373d53fb3e5c1959cc Mon Sep 17 00:00:00 2001 From: Martin Zurowietz Date: Sun, 29 Sep 2024 16:23:55 +0200 Subject: [PATCH 1/4] Improve metadata import These changes should make it easier to develop ner metadata import modules. --- app/Services/MetadataParsing/Annotation.php | 17 +++++++++++++++-- app/Services/MetadataParsing/FileMetadata.php | 3 ++- .../MetadataParsing/VolumeMetadataTest.php | 5 ++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/app/Services/MetadataParsing/Annotation.php b/app/Services/MetadataParsing/Annotation.php index 45d973e66..b9ec08bb4 100644 --- a/app/Services/MetadataParsing/Annotation.php +++ b/app/Services/MetadataParsing/Annotation.php @@ -6,7 +6,8 @@ use Biigle\Traits\HasPointsAttribute; use Exception; -class Annotation +// Abstract becaus it should not be used directly. +abstract class Annotation { use HasPointsAttribute; @@ -26,6 +27,13 @@ public function __construct( public array $labels, ) { $this->shape_id = $shape->id; + $this->setPointsAttribute($points); + + array_walk($labels, function ($label) { + if (!($label instanceof LabelAndUser)) { + throw new Exception('Annotation labels must be of class '.LabelAndUser::class); + } + }); } /** @@ -60,6 +68,11 @@ public function validate(): void */ public function setPointsAttribute(array $points) { - $this->points = array_map(fn ($coordinate) => round($coordinate, 2), $points); + $this->points = array_map( + fn ($a) => is_array($a) + ? array_map(fn ($b) => round($b, 2), $a) + : round($a, 2), + $points + ); } } diff --git a/app/Services/MetadataParsing/FileMetadata.php b/app/Services/MetadataParsing/FileMetadata.php index be9aa6895..0b8108ebd 100644 --- a/app/Services/MetadataParsing/FileMetadata.php +++ b/app/Services/MetadataParsing/FileMetadata.php @@ -2,7 +2,8 @@ namespace Biigle\Services\MetadataParsing; -class FileMetadata +// Abstract becaus it should not be used directly. +abstract class FileMetadata { /** * @var array diff --git a/tests/php/Services/MetadataParsing/VolumeMetadataTest.php b/tests/php/Services/MetadataParsing/VolumeMetadataTest.php index b26ba9233..ccd8648f9 100644 --- a/tests/php/Services/MetadataParsing/VolumeMetadataTest.php +++ b/tests/php/Services/MetadataParsing/VolumeMetadataTest.php @@ -4,7 +4,6 @@ use Biigle\Label as DbLabel; use Biigle\MediaType; -use Biigle\Services\MetadataParsing\FileMetadata; use Biigle\Services\MetadataParsing\ImageAnnotation; use Biigle\Services\MetadataParsing\ImageMetadata; use Biigle\Services\MetadataParsing\Label; @@ -30,7 +29,7 @@ public function testNew() public function testAddGetFiles() { $metadata = new VolumeMetadata; - $file = new FileMetadata('filename'); + $file = new ImageMetadata('filename'); $metadata->addFile($file); $this->assertEquals($file, $metadata->getFiles()[0]); $metadata->addFile($file); @@ -41,7 +40,7 @@ public function testGetFile() { $metadata = new VolumeMetadata; $this->assertNull($metadata->getFile('filename')); - $file = new FileMetadata('filename'); + $file = new ImageMetadata('filename'); $metadata->addFile($file); $this->assertEquals($file, $metadata->getFile('filename')); } From 8f5fe39d85cad0fb0eb41e03fa6bcab8cebb4bd5 Mon Sep 17 00:00:00 2001 From: Martin Zurowietz Date: Sun, 29 Sep 2024 20:38:44 +0200 Subject: [PATCH 2/4] Fix isEmpty definition for annotation-only imports --- app/Services/MetadataParsing/ImageMetadata.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/Services/MetadataParsing/ImageMetadata.php b/app/Services/MetadataParsing/ImageMetadata.php index 526314da1..3ea75a5e5 100644 --- a/app/Services/MetadataParsing/ImageMetadata.php +++ b/app/Services/MetadataParsing/ImageMetadata.php @@ -11,7 +11,8 @@ class ImageMetadata extends FileMetadata */ public function isEmpty(): bool { - return is_null($this->lat) + return !$this->hasAnnotations() + && is_null($this->lat) && is_null($this->lng) && is_null($this->takenAt) && is_null($this->area) From d05d5ff5a18d34643e0bb448a498414d77907ceb Mon Sep 17 00:00:00 2001 From: Martin Zurowietz Date: Mon, 30 Sep 2024 12:36:05 +0200 Subject: [PATCH 3/4] Improve chunked insert of metadata annotations Before this would execute an insert with the 0th annotation alone. --- app/Jobs/ImportVolumeMetadata.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Jobs/ImportVolumeMetadata.php b/app/Jobs/ImportVolumeMetadata.php index 4506c5bf5..02437caec 100644 --- a/app/Jobs/ImportVolumeMetadata.php +++ b/app/Jobs/ImportVolumeMetadata.php @@ -126,7 +126,7 @@ protected function insertAnnotations( // Insert in chunks because a single file can have tens of thousands of // annotations (e.g. a video or mosaic). - if (($index % static::$insertChunkSize) === 0) { + if ($index > 0 && ($index % static::$insertChunkSize) === 0) { $this->insertAnnotationChunk($file, $insertAnnotations, $insertAnnotationLabels); $insertAnnotations = []; $insertAnnotationLabels = []; From 7554d48ec67bb4c35ee5f6f474e02d77dece7429 Mon Sep 17 00:00:00 2001 From: Martin Zurowietz Date: Mon, 30 Sep 2024 12:36:36 +0200 Subject: [PATCH 4/4] Fix ordering of inserted annotation labels from metadata import --- app/Jobs/ImportVolumeMetadata.php | 1 + tests/php/Jobs/ImportVolumeMetadataTest.php | 56 +++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/app/Jobs/ImportVolumeMetadata.php b/app/Jobs/ImportVolumeMetadata.php index 02437caec..20eea4250 100644 --- a/app/Jobs/ImportVolumeMetadata.php +++ b/app/Jobs/ImportVolumeMetadata.php @@ -150,6 +150,7 @@ protected function insertAnnotationChunk( ->take(count($annotations)) ->pluck('id') ->reverse() + ->values() ->toArray(); foreach ($ids as $index => $id) { diff --git a/tests/php/Jobs/ImportVolumeMetadataTest.php b/tests/php/Jobs/ImportVolumeMetadataTest.php index 5fbbfaf73..d3eec4b71 100644 --- a/tests/php/Jobs/ImportVolumeMetadataTest.php +++ b/tests/php/Jobs/ImportVolumeMetadataTest.php @@ -532,4 +532,60 @@ public function testHandleChunkAnnotations() $this->assertEquals(2, $image->annotations()->count()); } + + /* + * This tests if the annotations and their labels match after the import. The check is + * important because annotations and labels are inserted in two different DB + * statements and the ordering is important. The ordering was mixed up at some point + * so the annotations did not get the correct labels. + */ + public function testHandleMultipleOrdering() + { + $image = Image::factory()->create(); + $dbUser = DbUser::factory()->create(); + $dbLabel = DbLabel::factory()->create(); + $dbLabel2 = DbLabel::factory()->create(); + + $metadata = new VolumeMetadata; + $file = new ImageMetadata($image->filename); + $metadata->addFile($file); + $label = new Label(123, 'my label'); + $user = new User(321, 'joe user'); + $lau = new LabelAndUser($label, $user); + $file->addFileLabel($lau); + $annotation = new ImageAnnotation( + shape: Shape::point(), + points: [10, 10], + labels: [$lau], + ); + $file->addAnnotation($annotation); + + $label2 = new Label(321, 'my other label'); + $lau2 = new LabelAndUser($label2, $user); + $annotation2 = new ImageAnnotation( + shape: Shape::point(), + points: [10, 10], + labels: [$lau2], + ); + $file->addAnnotation($annotation2); + + Cache::store('array')->put('metadata-pending-metadata-mymeta.csv', $metadata); + + $pv = PendingVolume::factory()->create([ + 'media_type_id' => MediaType::imageId(), + 'metadata_file_path' => 'mymeta.csv', + 'import_file_labels' => true, + 'import_annotations' => true, + 'user_map' => [321 => $dbUser->id], + 'label_map' => [123 => $dbLabel->id, 321 => $dbLabel2->id], + 'volume_id' => $image->volume_id, + ]); + + (new ImportVolumeMetadata($pv))->handle(); + + $annotations = $image->annotations()->orderBy('id', 'asc')->get(); + + $this->assertSame($dbLabel->id, $annotations[0]->labels[0]->label_id); + $this->assertSame($dbLabel2->id, $annotations[1]->labels[0]->label_id); + } }