Add to_structured_array to AcousticEncodingResultBase - #24
Conversation
to_structured_array to AcousticEncodingResultBase
There was a problem hiding this comment.
Pull request overview
This PR adds a to_structured_array method to AcousticEncodingResultBase for easier embedding handling, similar to the existing method in AcousticPredictionResultBase. The changes include refactoring shared constants to the base class and minor code cleanup.
Changes:
- Added
to_structured_array()method toAcousticEncodingResultBasefor converting embedding results to NumPy structured arrays - Moved shared constants (
VAR_INPUT,VAR_START_TIME,VAR_END_TIME) from specific result classes to the baseAcousticResultBaseclass - Added
_input_dtypeproperty toAcousticResultBaseto support polymorphic input type handling
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/birdnet/acoustic/inference/session.py | Removed unused variable is_file_input |
| src/birdnet/acoustic/inference/core/worker.py | Reordered imports alphabetically |
| src/birdnet/acoustic/inference/core/result_base.py | Added shared constants, _input_dtype property, and improved __exit__ type annotation |
| src/birdnet/acoustic/inference/core/prediction/prediction_worker.py | Reordered imports alphabetically |
| src/birdnet/acoustic/inference/core/prediction/prediction_result.py | Removed constants moved to base class and duplicate _input_dtype property |
| src/birdnet/acoustic/inference/core/encoding/encoding_tensor.py | Reordered imports alphabetically |
| src/birdnet/acoustic/inference/core/encoding/encoding_result.py | Added to_structured_array() method for embedding results |
| src/birdnet/acoustic/inference/core/consumer.py | Reordered imports alphabetically |
| pyproject.toml | Fixed grammar in test marker comment ('runned' → 'run') |
| .gitignore | Added playground.* pattern to ignore temporary development files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def to_structured_array(self) -> np.ndarray: | ||
| valid_mask_per_segment = ~(self._embeddings_masked).all(axis=2) | ||
| valid_file_idx, valid_seg_idx = np.where(valid_mask_per_segment) | ||
| n_embeddings = len(valid_file_idx) | ||
|
|
||
| embeddings_selected = self.embeddings[valid_file_idx, valid_seg_idx] | ||
|
|
||
| dtype = [ | ||
| (VAR_INPUT, self._input_dtype), | ||
| (VAR_START_TIME, self._input_durations.dtype), | ||
| (VAR_END_TIME, self._input_durations.dtype), | ||
| (VAR_EMBEDDING, self._embeddings.dtype, self.emd_dim), | ||
| ] | ||
|
|
||
| structured_array = np.empty(n_embeddings, dtype=dtype) | ||
| del dtype | ||
|
|
||
| if n_embeddings == 0: | ||
| return structured_array | ||
| del n_embeddings | ||
|
|
||
| sort_keys = ( | ||
| valid_seg_idx, | ||
| valid_file_idx, | ||
| ) | ||
| sort_indices = np.lexsort(sort_keys) | ||
| del sort_keys | ||
|
|
||
| file_idx_flat = valid_file_idx[sort_indices] | ||
| chunk_idx_flat = valid_seg_idx[sort_indices] | ||
| emb_flat = embeddings_selected[sort_indices] | ||
| del embeddings_selected | ||
| del sort_indices | ||
|
|
||
| hop_duration_s = get_hop_duration_s( | ||
| self._segment_duration_s[0], self._overlap_duration_s[0], self._speed[0] | ||
| ) | ||
| start_times = chunk_idx_flat.astype(self._input_durations.dtype) * hop_duration_s | ||
| del hop_duration_s | ||
| del chunk_idx_flat | ||
|
|
||
| structured_array[VAR_START_TIME] = start_times | ||
| structured_array[VAR_END_TIME] = np.minimum( | ||
| start_times | ||
| + apply_speed_to_duration(self._segment_duration_s[0], self._speed[0]), | ||
| self._input_durations[file_idx_flat], | ||
| ) | ||
| del start_times | ||
| structured_array[VAR_INPUT] = self._inputs[file_idx_flat] | ||
| del file_idx_flat | ||
|
|
||
| structured_array[VAR_EMBEDDING] = emb_flat | ||
| del emb_flat | ||
|
|
||
| return structured_array |
There was a problem hiding this comment.
The new to_structured_array method lacks test coverage. Looking at the codebase, the corresponding method in prediction_result.py has comprehensive test coverage in test_to_structured_array.py with tests for empty results, single predictions, unprocessable inputs, time calculations, overlap handling, speed factors, edge cases, and end-to-end tests. Similar test coverage should be added for this encoding result method to ensure correctness and maintain code quality standards.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…dnet-team/birdnet into structured-output-embeddings
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) as pbar: | ||
| for record in structured: | ||
| line = ( | ||
| f"{format_input_for_csv(record[VAR_INPUT])}," |
There was a problem hiding this comment.
The to_csv method uses the global format_input_for_csv function which always adds quotes around inputs. However, AcousticDataEncodingResult stores numeric array indices as inputs, not file paths. These numeric indices should not be quoted in CSV output. The _format_input_for_csv method defined in the subclass (line 325) is not being called, making it dead code.
Consider either:
- Calling
self._format_input_for_csv(...)instead of the globalformat_input_for_csv(...)to respect subclass-specific formatting, or - Removing the unused
_format_input_for_csvmethods from the subclasses if the current behavior is intentional.
| f"{format_input_for_csv(record[VAR_INPUT])}," | |
| f"{self._format_input_for_csv(record[VAR_INPUT])}," |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Thanks for the pull request. Next time please make pull requests to dev-branch. |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
to_structured_array,to_parquet,to_dataframe,to_pyarrowtoEncodeResultBaseAcousticResultBaseiterableencode_arraysandpredict_arraysto model classes