Remove kwargs and mutable defaults #48
Conversation
…`Instance` classes into separate modules within `io`
…`Instance` classes into separate modules within `io`
…nto aadi/refactor-data-structures
|
Warning Review failedThe pull request is closed. WalkthroughThe changes primarily involve updating import paths for Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Outside diff range and nitpick comments (15)
biogtr/io/__init__.py (5)
3-3: Consider addingFrameto__all__if it's intended for public use, or use it to avoid the static analysis warning.Tools
Ruff
3-3:
biogtr.io.frame.Frameimported but unused; consider removing, adding to__all__, or using a redundant alias
4-4: Consider addingInstanceto__all__if it's intended for public use, or use it to avoid the static analysis warning.Tools
Ruff
4-4:
biogtr.io.instance.Instanceimported but unused; consider removing, adding to__all__, or using a redundant alias
5-5: Consider addingAssociationMatrixto__all__if it's intended for public use, or use it to avoid the static analysis warning.Tools
Ruff
5-5:
biogtr.io.association_matrix.AssociationMatriximported but unused; consider removing, adding to__all__, or using a redundant alias
6-6: Consider addingTrackto__all__if it's intended for public use, or use it to avoid the static analysis warning.Tools
Ruff
6-6:
biogtr.io.track.Trackimported but unused; consider removing, adding to__all__, or using a redundant alias
8-8: Address or document the circular import issue mentioned in the TODO comment to ensure robust module architecture.biogtr/inference/track.py (1)
Line range hint
133-133: Remove unnecessary f-string or add placeholders as needed to avoid potential confusion or errors.- print(f"Pod Index: {index}") + print("Pod Index:", index)biogtr/models/global_tracking_transformer.py (1)
Line range hint
11-123: The refactoring ofGlobalTrackingTransformerto useInstanceobjects and returnAssociationMatrixobjects aligns with the project's new structure. However, ensureAssociationMatrixis correctly imported or defined to avoid runtime errors.+ from biogtr.io.association_matrix import AssociationMatrixbiogtr/datasets/microscopy_dataset.py (1)
Line range hint
87-87: Replace the lambda expression with a proper function definition to improve code clarity and maintainability.- parser = lambda x: data_utils.parse_synthetic(x, source=source) + def parser(x): + return data_utils.parse_synthetic(x, source=source)biogtr/datasets/cell_tracking_dataset.py (1)
126-126: Documentation update to reflect new module paths.Consider adding more detailed examples or scenarios to illustrate the use of
FrameandInstanceclasses in the context of this dataset.tests/test_inference.py (1)
Line range hint
168-168: Local variableNis assigned to but never used.- N = N_t * TRemove the unused variable to clean up the code.
Tools
Ruff
4-4:
pytestimported but unusedbiogtr/inference/metrics.py (1)
108-108: Documentation update to reflect new module paths.Consider adding more detailed examples or scenarios to illustrate the use of
Frameclass in the context of MOT metrics.tests/test_models.py (1)
Line range hint
429-429: Remove unused variable:img_shape.- img_shape = (1, 100, 100)biogtr/inference/tracker.py (2)
131-131: Update documentation to reflect new import paths.Consider updating the documentation to reflect the new import paths accurately. This helps maintain clarity and consistency in the codebase.
Line range hint
177-177: Fix undefined variable and remove unused variable.- curr_track = instance.pred_track_id + curr_track_id = instance.pred_track_idbiogtr/io/frame.py (1)
74-93: Review string representation for completeness and accuracy.Consider reviewing the string representation method
__repr__to ensure it includes all relevant attributes and provides a clear and accurate representation of theFrameobject.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (26)
- biogtr/datasets/base_dataset.py (1 hunks)
- biogtr/datasets/cell_tracking_dataset.py (2 hunks)
- biogtr/datasets/eval_dataset.py (1 hunks)
- biogtr/datasets/microscopy_dataset.py (2 hunks)
- biogtr/datasets/sleap_dataset.py (1 hunks)
- biogtr/inference/metrics.py (2 hunks)
- biogtr/inference/track.py (1 hunks)
- biogtr/inference/track_queue.py (1 hunks)
- biogtr/inference/tracker.py (9 hunks)
- biogtr/io/init.py (1 hunks)
- biogtr/io/association_matrix.py (1 hunks)
- biogtr/io/config.py (3 hunks)
- biogtr/io/frame.py (1 hunks)
- biogtr/io/instance.py (1 hunks)
- biogtr/io/track.py (1 hunks)
- biogtr/models/global_tracking_transformer.py (4 hunks)
- biogtr/models/gtr_runner.py (8 hunks)
- biogtr/models/model_utils.py (1 hunks)
- biogtr/models/transformer.py (8 hunks)
- biogtr/training/losses.py (2 hunks)
- biogtr/training/train.py (1 hunks)
- tests/test_config.py (1 hunks)
- tests/test_data_model.py (4 hunks)
- tests/test_inference.py (1 hunks)
- tests/test_models.py (6 hunks)
- tests/test_training.py (1 hunks)
Files skipped from review due to trivial changes (1)
- biogtr/inference/track_queue.py
Additional context used
Ruff
biogtr/io/__init__.py
3-3:
biogtr.io.frame.Frameimported but unused; consider removing, adding to__all__, or using a redundant alias
4-4:
biogtr.io.instance.Instanceimported but unused; consider removing, adding to__all__, or using a redundant alias
5-5:
biogtr.io.association_matrix.AssociationMatriximported but unused; consider removing, adding to__all__, or using a redundant alias
6-6:
biogtr.io.track.Trackimported but unused; consider removing, adding to__all__, or using a redundant aliasbiogtr/io/track.py
17-17: Undefined name
Instance
46-46: Undefined name
Instances
64-64: Undefined name
Frame
80-80: Undefined name
Instance
80-80: Undefined name
Instancetests/test_training.py
10-10:
biogtr.models.global_tracking_transformer.GlobalTrackingTransformerimported but unused
11-11:
omegaconf.DictConfigimported but unused
47-47: Local variable
featsis assigned to but never usedbiogtr/inference/track.py
133-133: f-string without any placeholders
biogtr/models/global_tracking_transformer.py
83-83: Undefined name
AssociationMatrixbiogtr/datasets/microscopy_dataset.py
87-87: Do not assign a
lambdaexpression, use adefbiogtr/models/model_utils.py
3-3:
typing.Tupleimported but unusedtests/test_inference.py
4-4:
pytestimported but unused
168-168: Local variable
Nis assigned to but never usedbiogtr/datasets/sleap_dataset.py
9-9:
warningsimported but unusedtests/test_models.py
429-429: Local variable
img_shapeis assigned to but never usedbiogtr/inference/tracker.py
121-121: f-string without any placeholders
177-177: Undefined name
curr_track
177-177: Local variable
curr_trackis assigned to but never usedbiogtr/io/frame.py
59-59: Undefined name
AssociationMatrix
328-328: Undefined name
AssociationMatrix
347-347: Undefined name
AssociationMatrixbiogtr/io/instance.py
93-93: Undefined name
Frame
520-520: Undefined name
Frame
529-529: Undefined name
Frame
Additional comments not posted (46)
biogtr/datasets/eval_dataset.py (3)
4-4: The import ofFrameis correctly updated to reflect the new module path.
5-5: The import ofInstanceis correctly updated to reflect the new module path.
Line range hint
10-37: The implementation ofEvalDatasetclass is robust, handling datasets merging and index errors effectively.tests/test_config.py (2)
4-4: The import ofConfigis correctly updated to reflect the new module path.
Line range hint
14-73: The test functions intest_config.pyare comprehensive, covering initialization, setters, and getters of theConfigclass effectively.biogtr/io/track.py (3)
3-3: The import ofattrsis correctly used for defining theTrackclass.
4-4: The import ofUnionfromtypingis correctly used for type hinting in theTrackclass.
7-94: The implementation of theTrackclass is robust, with well-defined properties and methods for managing track instances.Tools
Ruff
17-17: Undefined name
Instance
46-46: Undefined name
Instances
64-64: Undefined name
Frame
80-80: Undefined name
Instance
80-80: Undefined name
Instancebiogtr/training/train.py (2)
6-6: The import ofConfigis correctly updated to reflect the new module path.
Line range hint
18-86: Themainfunction intrain.pyis well-implemented, handling training configuration, dataset preparation, and model training effectively.tests/test_training.py (4)
6-6: The import ofFrameis correctly updated to reflect the new module path.
7-7: The import ofInstanceis correctly updated to reflect the new module path.
12-12: The import ofConfigis correctly updated to reflect the new module path.
Line range hint
19-112: The test functions intest_training.pyare comprehensive, covering loss calculation and model runner configuration effectively.Tools
Ruff
10-10:
biogtr.models.global_tracking_transformer.GlobalTrackingTransformerimported but unused
11-11:
omegaconf.DictConfigimported but unusedbiogtr/inference/track.py (2)
3-5: Updated import paths look correct and align with the project's restructuring.
Line range hint
10-32: Theexport_trajectoriesfunction is well-implemented, efficiently handling data extraction and optional saving.biogtr/datasets/base_dataset.py (1)
4-4: Updated import path forFrameis correct and aligns with the project's restructuring.tests/test_data_model.py (1)
3-10: The imports for testingFrame,Instance,AssociationMatrix, andTrackare correctly updated to reflect the new module paths.biogtr/datasets/microscopy_dataset.py (1)
6-7: Updated import paths forFrameandInstanceare correct and align with the project's restructuring.biogtr/datasets/cell_tracking_dataset.py (1)
6-7: Updated import paths forFrameandInstanceclasses.tests/test_inference.py (1)
6-7: Updated import paths forFrameandInstanceclasses.biogtr/training/losses.py (1)
3-3: Updated import path forFrameclass.biogtr/models/gtr_runner.py (1)
11-12: Updated import paths forFrameandInstanceclasses.biogtr/inference/metrics.py (1)
6-6: Updated import path forFrameclass.biogtr/io/config.py (1)
81-85: The handling ofckpt_pathinget_modelmethod is implemented correctly. It ensures that the model is loaded from a checkpoint if provided, which aligns with the PR objectives to manage model loading more effectively.biogtr/io/association_matrix.py (8)
25-42: LGTM! Proper validation for reference instances against matrix columns.
44-61: LGTM! Proper validation for query instances against matrix rows.
75-83: LGTM! Efficient handling of data type conversion for the matrix.
85-131: LGTM! Flexible and correct implementation of DataFrame conversion.
133-196: LGTM! Correct and detailed implementation of matrix reduction with comprehensive error handling.
198-221: LGTM! Robust implementation of matrix element retrieval with comprehensive error handling.
223-261: LGTM! Robust type checking and index retrieval implementation.
263-302: LGTM! Flexible and correct implementation of instance grouping by track ID.biogtr/datasets/sleap_dataset.py (1)
10-11: LGTM! Correct import statements forFrameandInstance.biogtr/inference/tracker.py (2)
6-6: Update import paths to reflect new module structure.
284-284: Ensure that the association matrix is correctly handled.Verification successful
The search results indicate that
asso_outputis used in various parts of the codebase, including tests and multiple modules. This suggests that the association matrix is indeed being handled and integrated into the frame data structure.
tests/test_data_model.py: Multiple assertions and assignments involvingasso_output.biogtr/models/model_utils.py: Functions manipulatingasso_output.biogtr/io/frame.py: Getter and setter methods forasso_output.biogtr/inference/post_processing.py: Functions processingasso_output.biogtr/inference/tracker.py: Assignments and manipulations ofasso_output.These usages indicate that
asso_outputis being actively used and tested, which supports the notion that it is correctly handled.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the association matrix is correctly handled and integrated into the frame data structure. # Test: Search for usage of `asso_output` in the codebase. Expect: Correct handling and integration. rg --type python $'asso_output'Length of output: 61
Script:
#!/bin/bash # Description: Verify that the association matrix is correctly handled and integrated into the frame data structure. # Test: Search for usage of `asso_output` in the codebase. Expect: Correct handling and integration. rg --type py 'asso_output'Length of output: 5904
biogtr/io/frame.py (3)
51-62: Ensure correct use of data types and default values.Verification successful
The search results show the attribute definitions in the
Frameclass, confirming the data types and default values. Here are the relevant lines frombiogtr/io/frame.py:
_video_id: int = attrs.field(alias="video_id", converter=_to_tensor)_frame_id: int = attrs.field(alias="frame_id", converter=_to_tensor)_video: str = attrs.field(alias="vid_file", default="")_img_shape: ArrayLike = attrs.field(alias="img_shape", converter=_to_tensor, factory=list)_instances: list["Instance"] = attrs.field(alias="instances", factory=list)_asso_output: "AssociationMatrix" = attrs.field(alias="asso_output", default=None)_matches: tuple = attrs.field(alias="matches", factory=tuple)_traj_score: dict = attrs.field(alias="traj_score", factory=dict)_device: str = attrs.field(alias="device", default=None)These definitions appear to use correct data types and sensible default values.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the data types and default values are correctly used in the `Frame` class. # Test: Search for attribute definitions in the `Frame` class. Expect: Correct data types and sensible default values. rg --type python $'attrs.field'Length of output: 61
Script:
#!/bin/bash # Description: Verify that the data types and default values are correctly used in the `Frame` class. # Test: Search for attribute definitions in the `Frame` class. Expect: Correct data types and sensible default values. rg 'attrs.field' --glob '*.py'Length of output: 2391
Tools
Ruff
59-59: Undefined name
AssociationMatrix
95-123: Ensure device handling is robust and error-free.
125-157: Validate conversion logic fromsio.LabeledFrametoFrame.Verification successful
The
from_slpmethod inbiogtr/io/frame.pyappears to handle the conversion fromsio.LabeledFrametoFramecorrectly, including edge cases for image shapes and data types. The method processes the image shape, frame index, video filename, and instances appropriately.
- The method checks the shape of the image and adjusts it if necessary.
- It converts the frame index to
np.int32if it is a number.- It extracts the video filename.
- It converts instances using the
Instance.from_slpmethod.Based on the provided code, the conversion logic seems robust and handles various edge cases.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Validate the conversion logic from `sio.LabeledFrame` to `Frame` to ensure it handles all edge cases and data types correctly. # Test: Review the `from_slp` class method. Expect: Correct handling of all data types and edge cases. rg --type python $'from_slp'Length of output: 58
Script:
#!/bin/bash # Description: Locate and review the `from_slp` class method in the codebase to ensure it handles all edge cases and data types correctly. # Locate the `from_slp` method in the codebase ast-grep --lang python --pattern $'class $_ { $$$ @classmethod def from_slp($_, $_, $_=0, $_=None, **$_) -> $_: $$$ $$$ }'Length of output: 656
Script:
#!/bin/bash # Description: Locate and review the `from_slp` class method in the codebase to ensure it handles all edge cases and data types correctly. # Locate the `from_slp` method in the codebase ast-grep --lang python --pattern $'@classmethod def from_slp($_, $_, $_=0, $_=None, **$_) -> $_: $$$ 'Length of output: 105
Script:
#!/bin/bash # Description: Locate and review the `from_slp` class method in the codebase to ensure it handles all edge cases and data types correctly. # Locate the `from_slp` method in the codebase rg --python 'def from_slp'Length of output: 439
Script:
#!/bin/bash # Description: Locate and review the `from_slp` class method in the codebase to ensure it handles all edge cases and data types correctly. # Locate the `from_slp` method in the codebase rg 'def from_slp' --glob '*.py'Length of output: 110
Script:
#!/bin/bash # Description: Extract the full implementation of the `from_slp` method in `biogtr/io/frame.py` to ensure it handles all edge cases and data types correctly. # Extract the `from_slp` method implementation ast-grep --lang python --pattern $'class $_ { $$$ @classmethod def from_slp($_, $_, $_=0, $_=None, **$_) -> $_: $$$ $$$ }' biogtr/io/frame.pyLength of output: 675
Script:
#!/bin/bash # Description: Extract the full implementation of the `from_slp` method in `biogtr/io/frame.py` to ensure it handles all edge cases and data types correctly. # Extract the `from_slp` method implementation ast-grep --lang python --pattern $'@classmethod def from_slp($_, $_, $_=0, $_=None, **$_) -> $_: $$$ ' biogtr/io/frame.pyLength of output: 124
Script:
#!/bin/bash # Description: Locate and review the `from_slp` class method in the codebase to ensure it handles all edge cases and data types correctly. # Locate the `from_slp` method in the codebase rg 'def from_slp' biogtr/io/frame.pyLength of output: 56
Script:
#!/bin/bash # Description: Display the content of `biogtr/io/frame.py` to review the `from_slp` method implementation. # Display the content of `biogtr/io/frame.py` cat biogtr/io/frame.pyLength of output: 18363
biogtr/models/transformer.py (7)
14-15: Updated imports align with the new module structure.Also applies to: 18-18
144-145: The modifications in theforwardmethod to useInstanceandAssociationMatrixare consistent with the PR objectives.Also applies to: 149-150, 158-160, 162-165, 167-169, 171-175, 178-180, 182-182, 184-184, 186-186, 188-190, 194-194, 197-197, 202-206, 208-212, 214-216, 218-218, 220-220, 222-222, 224-226, 228-228, 230-231, 233-235, 238-241, 253-254, 256-258, 261-261
261-261: TheTransformerEncoderLayerclass is well-implemented and documented.
384-385: Positional embeddings are correctly applied in the encoder layers.Also applies to: 393-393, 401-402, 405-405, 531-531
384-385: TheTransformerDecoderLayerclass is well-implemented and documented.Also applies to: 393-393, 401-402, 405-405, 531-531
384-385: Positional embeddings are correctly applied in the decoder layers.Also applies to: 393-393, 401-402, 405-405, 531-531
384-385: Ensure positional embeddings are correctly handled in the decoder layers.Also applies to: 393-393, 401-402, 405-405, 531-531
Verification successful
Ensure positional embeddings are correctly handled in the decoder layers.
The search results indicate that
ref_pos_embandquery_pos_embare used in various parts of thebiogtr/models/transformer.pyfile, including their initialization, addition to embeddings, and usage in encoder and decoder layers. This suggests that positional embeddings are being handled appropriately.
- Lines where
ref_pos_embandquery_pos_embare used:
- Initialization:
ref_pos_emb = self.pos_emb(ref_boxes),query_pos_emb = self.pos_emb(query_boxes)- Addition to embeddings:
instance.add_embedding("pos", ref_pos_emb[i]),instance.add_embedding("pos", query_pos_emb[i])- Usage in encoder and decoder layers:
encoder_features = encoder_features + ref_pos_emb,decoder_queries = decoder_queries + query_pos_embScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that positional embeddings are correctly handled in the decoder layers. # Test: Search for the usage of positional embeddings in the decoder layers. Expect: Correct handling of positional embeddings. rg --type python $'ref_pos_emb|query_pos_emb' biogtr/models/transformer.pyLength of output: 106
Script:
#!/bin/bash # Description: Verify that positional embeddings are correctly handled in the decoder layers. # Test: Search for the usage of positional embeddings in the decoder layers. Expect: Correct handling of positional embeddings. rg 'ref_pos_emb|query_pos_emb' biogtr/models/transformer.pyLength of output: 1469
biogtr/io/config.py
Outdated
| ckpt_path = model_params.pop("ckpt_path", None) | ||
|
|
||
| if self.cfg.model.ckpt_path is not None and self.cfg.model.ckpt_path != "": | ||
| if ckpt_path is not None and ckpt_path != "": |
There was a problem hiding this comment.
There is a potential bug in the get_gtr_runner method. The use of self.cfg.model.ckpt_path after it has potentially been removed by pop could lead to an AttributeError. Consider using the local variable ckpt_path instead.
- self.cfg.model.ckpt_path,
+ ckpt_path,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.
| ckpt_path = model_params.pop("ckpt_path", None) | |
| if self.cfg.model.ckpt_path is not None and self.cfg.model.ckpt_path != "": | |
| if ckpt_path is not None and ckpt_path != "": | |
| ckpt_path = model_params.pop("ckpt_path", None) | |
| if ckpt_path is not None and ckpt_path != "": | |
| ckpt_path, |
biogtr/io/instance.py
Outdated
| self._embeddings[emb_type] = embedding | ||
|
|
||
| @property | ||
| def frame(self) -> "Frame": |
There was a problem hiding this comment.
Undefined reference to Frame in type hints and setter.
The type hints and setter method for frame reference Frame, which is not defined in this file. This could lead to runtime errors if not addressed. Ensure that Frame is properly imported or defined.
Also applies to: 529-529
Tools
Ruff
520-520: Undefined name
Frame
biogtr/io/instance.py
Outdated
| _skeleton: sio.Skeleton = attrs.field(alias="skeleton", default=None) | ||
| _pose: dict[str, ArrayLike] = attrs.field(alias="pose", factory=dict) | ||
| _device: str = attrs.field(alias="device", default=None) | ||
| _frame: "Frame" = None |
There was a problem hiding this comment.
Undefined reference to Frame.
The type hint for _frame references Frame, which is not defined in this file. Ensure that Frame is imported or defined elsewhere in the project.
Tools
Ruff
93-93: Undefined name
Frame
biogtr/inference/tracker.py
Outdated
| asso_matrix = model(all_instances, query_instances) | ||
| # if model.transformer.return_embedding: | ||
| # query_frame.embeddings = embed TODO add embedding to Instance Object | ||
| # if query_frame == 1: | ||
| # print(asso_output) | ||
| asso_output = asso_output[-1].split( | ||
| asso_output = asso_matrix[-1].matrix.split( |
There was a problem hiding this comment.
Optimize tensor operations and ensure correct usage of API.
Consider optimizing tensor operations and ensuring that the API usage is correct. For instance, verify that the split method is used appropriately and that the tensor dimensions align as expected.
biogtr/inference/tracker.py
Outdated
| @@ -207,7 +209,7 @@ | |||
|
|
|||
| Args: | |||
| model: the pretrained GlobalTrackingTransformer to be used for inference | |||
| frames: A list of Frames containing reid features. See `biogtr.data_structures` for more info. | |||
| frames: A list of Frames containing reid features. See `biogtr.io.data_structures` for more info. | |||
There was a problem hiding this comment.
Refactor to improve clarity and reduce redundancy.
Consider refactoring the code to improve clarity and reduce redundancy. For example, the list comprehension for all_instances can be simplified or explained better to enhance readability.
Also applies to: 237-239
biogtr/inference/tracker.py
Outdated
| tracked_frames = self.track_queue.collate_tracks( | ||
| device=frame_to_track.frame_id.device | ||
| ) |
There was a problem hiding this comment.
Tip
Codebase Verification
Ensure device compatibility for tensor operations.
The search results indicate several instances where tensors are created without specifying a device. This could lead to potential issues if the tensors are used in operations that require device compatibility.
biogtr/io/frame.pybiogtr/io/instance.pybiogtr/models/model_utils.pybiogtr/datasets/sleap_dataset.pybiogtr/inference/boxes.pybiogtr/models/embedding.pytests/test_data_model.pytests/test_inference.py
Please review these instances and ensure that the device is specified where necessary.
Analysis chain
Ensure device compatibility for tensor operations.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all tensor operations are compatible with the specified device.
# Test: Search for tensor operations that might not specify device. Expect: All operations specify the device.
rg --type python $'tensor\('
Length of output: 60
Script:
#!/bin/bash
# Description: Verify that all tensor operations are compatible with the specified device.
# Test: Search for tensor operations that might not specify device. Expect: All operations specify the device.
rg --type py $'tensor\('
Length of output: 2528
biogtr/io/frame.py
Outdated
| ) | ||
|
|
||
| _instances: list["Instance"] = attrs.field(alias="instances", factory=list) | ||
| _asso_output: "AssociationMatrix" = attrs.field(alias="asso_output", default=None) |
There was a problem hiding this comment.
Define or import AssociationMatrix before usage.
It appears that AssociationMatrix is used but not defined or imported in this file. Ensure that AssociationMatrix is correctly imported from its module to avoid runtime errors.
Also applies to: 328-328, 347-347
Tools
Ruff
59-59: Undefined name
AssociationMatrix
tests/test_inference.py
Outdated
| @@ -3,13 +3,93 @@ | |||
| import torch | |||
| import pytest | |||
There was a problem hiding this comment.
pytest imported but unused.
Consider removing the unused import to clean up the code.
Tools
Ruff
4-4:
pytestimported but unused
biogtr/datasets/sleap_dataset.py
Outdated
| @@ -7,7 +7,8 @@ | |||
| import sleap_io as sio | |||
| import random | |||
| import warnings | |||
There was a problem hiding this comment.
Remove unused import: warnings.
- import warningsCommittable 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.
| import warnings |
Tools
Ruff
9-9:
warningsimported but unused
biogtr/models/model_utils.py
Outdated
| @@ -2,35 +2,69 @@ | |||
|
|
|||
| from typing import List, Tuple, Iterable | |||
There was a problem hiding this comment.
Remove the unused import to clean up the code.
- from typing import List, Tuple, Iterable
+ from typing import List, IterableCommittable 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.
| from typing import List, Tuple, Iterable | |
| from typing import List, Iterable |
Tools
Ruff
3-3:
typing.Tupleimported but unused
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
biogtr/models/global_tracking_transformer.py (1)
Line range hint
83-83: Undefined nameAssociationMatrixin the return type of theforwardmethod.- from biogtr.io.instance import Instance + from biogtr.io.instance import Instance + from biogtr.io.association_matrix import AssociationMatrixbiogtr/io/association_matrix.py (1)
Line range hint
180-180: Add missingselfparameter in thereducemethod.- def reduce( + def reduce(self,Also applies to: 181-181, 183-183, 184-184, 186-186, 187-187, 190-190, 190-190, 195-195, 195-195, 202-202
biogtr/inference/tracker.py (1)
Line range hint
176-176: Initialize thecurr_trackvariable before use.+ curr_track = 0 for i, instance in enumerate(frames[batch_idx].instances): instance.pred_track_id = instance.gt_track_id curr_track = instance.pred_track_id
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- biogtr/inference/tracker.py (1 hunks)
- biogtr/io/association_matrix.py (6 hunks)
- biogtr/io/config.py (2 hunks)
- biogtr/models/global_tracking_transformer.py (1 hunks)
- biogtr/models/gtr_runner.py (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- biogtr/io/config.py
- biogtr/models/gtr_runner.py
Additional context used
Ruff
biogtr/models/global_tracking_transformer.py
83-83: Undefined name
AssociationMatrixbiogtr/io/association_matrix.py
113-113: f-string without any placeholders
137-137: f-string without any placeholders
180-180: Undefined name
self
181-181: Undefined name
self
183-183: Undefined name
self
184-184: Undefined name
self
186-186: Undefined name
self
187-187: Undefined name
self
190-190: Undefined name
self
190-190: Undefined name
self
195-195: Undefined name
self
195-195: Undefined name
self
202-202: Undefined name
selfbiogtr/inference/tracker.py
121-121: f-string without any placeholders
176-176: Undefined name
curr_track
176-176: Local variable
curr_trackis assigned to but never used
As per #40 and #43, mutable defaults are very dangerous and
kwargsare opaque. Thus we should never use mutable defaults and only usekwargswhen absolutely necessary (i.e. for downstream function params). Thus here we remove any usage of mutable defaults and remove the use of kwargs from `GlobalTrackingTransformerSummary by CodeRabbit
Refactor
FrameandInstanceclasses across multiple modules for better modularity and code organization.GlobalTrackingTransformerto handle lists ofInstanceobjects instead ofFrameobjects.New Features
AssociationMatrixclass for managing association scores between detections.FrameandInstanceclasses.Documentation