basic python api #31
basic python api #31
Conversation
WalkthroughWell, lemme tell ya, we’ve been grindin’ gears to whip this code into shape! We tightened up those configs, cleaned out the clutter, and made sure everything runs smoother than a greased axle. Tossed in some shiny new parameters and revamped them scripts like they were long overdue for a tune-up. Buckle up, ‘cause this ol’ machine’s ready to roll outta the yard! Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Model
participant Config
User->>CLI: Start processing
CLI->>Config: Load configuration
Config-->>CLI: Configuration loaded
CLI->>Model: Execute model with data
Model-->>CLI: Process results
CLI-->>User: Display results
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 Configuration 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: 4
Outside diff range, codebase verification and nitpick comments (15)
stac_mjx/compute_stac.py (3)
12-13: Swap them print statements for proper logging, partner.Using print statements for debugging ain't the best practice. Swap 'em out for proper logging to keep things tidy.
- print("Root Optimization:") + logger.info("Root Optimization:")
Line range hint
71-72:
Swap them print statements for proper logging, partner.Using print statements for debugging ain't the best practice. Swap 'em out for proper logging to keep things tidy.
- print("Begining offset optimization:") + logger.info("Beginning offset optimization:")
Line range hint
132-133:
Swap them print statements for proper logging, partner.Using print statements for debugging ain't the best practice. Swap 'em out for proper logging to keep things tidy.
- print("Pose Optimization:") + logger.info("Pose Optimization:")stac_mjx/controller.py (6)
203-204: Swap them print statements for proper logging, partner.Using print statements for debugging ain't the best practice. Swap 'em out for proper logging to keep things tidy.
- print(f"Calibration iteration: {n_iter + 1}/{utils.params['N_ITERS']}") + logger.info(f"Calibration iteration: {n_iter + 1}/{utils.params['N_ITERS']}")
Line range hint
203-255:
Add a docstring for thefitfunction, partner.The function signature and return type should be documented to improve readability and maintainability.
def fit(mj_model, kp_data): """ Perform pose optimization. Args: mj_model (mujoco.Model): MJX Model kp_data (jnp.Array): Keypoint data Returns: Tuple: Optimized model, joint angles, positions, and keypoint data """
286-287: Swap them print statements for proper logging, partner.Using print statements for debugging ain't the best practice. Swap 'em out for proper logging to keep things tidy.
- print("Final pose optimization") + logger.info("Final pose optimization")
Line range hint
286-302:
Add a docstring for themjx_setupfunction, partner.The function signature and return type should be documented to improve readability and maintainability.
def mjx_setup(kp_data, mj_model): """ Create MJX model and data, and set offsets. Args: kp_data (jnp.Array): Keypoint data mj_model (mujoco.Model): MJX Model Returns: Tuple: MJX Model and Data """
304-305: Swap them print statements for proper logging, partner.Using print statements for debugging ain't the best practice. Swap 'em out for proper logging to keep things tidy.
- print(f"shape of qpos: {q.shape}") + logger.info(f"shape of qpos: {q.shape}")
304-332: Add a docstring for thepackage_datafunction, partner.The function signature and return type should be documented to improve readability and maintainability.
def package_data(mjx_model, physics, q, x, walker_body_sites, kp_data, batched=False): """ Extract and package various parameters related to the model's pose and offsets. Args: mjx_model (mujoco.Model): MJX Model physics (dmcontrol.Physics): Physics model q (jnp.Array): Joint angles x (jnp.Array): Positions walker_body_sites (jnp.Array): Walker body sites kp_data (jnp.Array): Keypoint data batched (bool, optional): Whether the data is batched. Defaults to False. Returns: dict: Packaged data """use_api.ipynb (3)
1-18: Add comments to explain the imports, partner.Adding comments to explain the purpose of each import will improve readability and maintainability.
from stac_mjx import main # Main module for STAC from stac_mjx import utils # Utility functions for STAC from jax import numpy as jp # JAX NumPy for numerical operations import numpy as np # NumPy for numerical operations stac_config_path = "./configs/stac.yaml" # Path to STAC configuration model_config_path = "./configs/rodent.yaml" # Path to model configuration
28-34: Add comments to explain the steps, partner.Adding comments to explain the purpose of each step will improve readability and maintainability.
# Load configurations using the main module cfg = main.load_configs(stac_config_path, model_config_path)
44-63: Add comments to explain the steps, partner.Adding comments to explain the purpose of each step will improve readability and maintainability.
data_path = "./save_data_avg.mat" # Path to the data file # Set up mocap data kp_names = utils.params["KP_NAMES"] # argsort returns the indices that would sort the array stac_keypoint_order = np.argsort(kp_names) data_path = cfg.paths.data_path # Load kp_data, /1000 to scale data (from mm to meters) kp_data = utils.loadmat(data_path)["pred"][:] / 1000 # Preparing data by reordering and reshaping (TODO: will this stay the same?) # Resulting kp_data is of shape (n_frames, n_keypoints) kp_data = jp.array(kp_data[:, :, stac_keypoint_order]) kp_data = jp.transpose(kp_data, (0, 2, 1)) kp_data = jp.reshape(kp_data, (kp_data.shape[0], -1))stac_mjx/viz.py (3)
Line range hint
96-189:
This function's longer than a Texas mile.This
overlay_vizfunction's doin' a whole lotta heavy liftin'. Might be a good idea to break it down into smaller helper functions for better readability and maintainability. Keep it simple, partner.
Line range hint
192-261:
Could use some more road signs here.This
mujoco_vizfunction's got a good structure, but it could use some more comments to explain what's goin' on at each step. Don't leave folks guessin'.
Line range hint
264-340:
More comments needed, partner.This
mujoco_pair_vizfunction's doin' some good work, but it could use more comments to explain the key steps. Help folks understand the journey.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
videos/direct_render.mp4is excluded by!**/*.mp4
Files selected for processing (15)
- configs/rodent.yaml (2 hunks)
- configs/stac.yaml (1 hunks)
- environment.yaml (1 hunks)
- requirements.txt (1 hunks)
- run_rodent.py (1 hunks)
- setup.py (1 hunks)
- stac_mjx/init.py (1 hunks)
- stac_mjx/compute_stac.py (2 hunks)
- stac_mjx/controller.py (5 hunks)
- stac_mjx/main.py (4 hunks)
- stac_mjx/operations.py (1 hunks)
- stac_mjx/stac_base.py (8 hunks)
- stac_mjx/utils.py (2 hunks)
- stac_mjx/viz.py (1 hunks)
- use_api.ipynb (1 hunks)
Files skipped from review due to trivial changes (5)
- configs/stac.yaml
- setup.py
- stac_mjx/init.py
- stac_mjx/operations.py
- stac_mjx/stac_base.py
Additional context used
yamllint
environment.yaml
[error] 17-17: trailing spaces
(trailing-spaces)
[warning] 18-18: wrong indentation: expected 6 but found 4
(indentation)
[error] 18-18: trailing spaces
(trailing-spaces)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
configs/rodent.yaml
[error] 4-4: trailing spaces
(trailing-spaces)
[error] 9-9: trailing spaces
(trailing-spaces)
Additional comments not posted (22)
requirements.txt (1)
16-16: Good job on adding the version constraint!Ensuring
optax[all]is at least version 0.2.3 helps keep things compatible and avoids any nasty surprises down the road.environment.yaml (1)
1-22: Overall, this here file looks good!The dependencies and environment variables are all set up correctly. Just fix them formatting issues and you'll be good to go.
Tools
yamllint
[error] 17-17: trailing spaces
(trailing-spaces)
[warning] 18-18: wrong indentation: expected 6 but found 4
(indentation)
[error] 18-18: trailing spaces
(trailing-spaces)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
run_rodent.py (4)
1-3: Imports look solid!You got the right imports for JAX and numpy. Good to go!
6-10: Imports and logging setup look good!You're all set with the OS, logging, and Hydra imports.
11-12: STAC imports are A-OK!Got the main and utils from STAC. No problems here.
50-54: Main execution block looks good!Logging the results after running the main function. Looks solid!
stac_mjx/utils.py (1)
58-60: Good job on the type hinting, partner!Adding type hinting for
cfgmakes it clearer what kind of data we're dealing with. Keep on truckin'!stac_mjx/main.py (5)
17-30: Loadin' configs like a pro!The
load_configsfunction is well-documented and gets the job done right. Keep on haulin'!
33-45: Solid docstring and function signature, partner!The updated function signature and detailed docstring make it clear what this function does. Nice work!
71-76: Good use of logging and random sampling!Logging the shape of
kp_dataand usingjax.random.choicefor sampling frames is a smart move. Keep on truckin'!
89-91: Clear and informative logging!Logging the skip message for the transform step helps users understand what's going on. Nice touch!
108-110: Efficient logging and saving!Logging the time taken and saving the transform data efficiently. Good job, partner!
configs/rodent.yaml (3)
2-2: Check the frame count, partner!Reducin'
N_FRAMES_PER_CLIPfrom 360 to 250 might impact performance or output quality. Make sure this change fits the bill.
5-5: Tightenin' the tolerance, eh?Changin'
FTOLto 5.0e-03 might improve model accuracy. Make sure this change aligns with the project goals.
10-10: What's with the redundancy, partner?Reiteratin'
N_ITERSmight be for clarity, but make sure it's intentional and doesn't cause confusion.use_api.ipynb (2)
21-25: LGTM!This markdown cell looks good to me.
37-41: LGTM!This markdown cell looks good to me.
stac_mjx/viz.py (5)
16-16: Good move on switchin' to absolute imports.This here change makes it clearer where the
utilsmodule's comin' from. Keepin' things tidy and easy to maintain.
17-17: New import forcontrollerlooks solid.Looks like you're bringin' in
controllerfor some new tricks. No complaints here.
Line range hint
67-81:
Clear as a bell.This
convert_camerafunction's lookin' good. Clear and concise. No issues here.
Line range hint
84-98:
Lookin' sharp.This
convert_camera_indivfunction's clear and to the point. No issues here.
Line range hint
101-109:
Nice and tidy.This
convert_camerasfunction's doin' its job well. No issues here.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
==========================================
+ Coverage 6.16% 29.71% +23.54%
==========================================
Files 9 9
Lines 665 488 -177
==========================================
+ Hits 41 145 +104
+ Misses 624 343 -281 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .gitignore (1 hunks)
- tests/stac_mjx/test_main.py (1 hunks)
Files skipped from review due to trivial changes (1)
- .gitignore
Additional comments not posted (4)
tests/stac_mjx/test_main.py (4)
1-5: Imports look good!The imports are necessary and appropriate for the test file.
8-17: Fixtures look good!The
mock_omegaconf_loadfixture correctly mocksOmegaConf.loadwith the expected side effects.
20-24: Fixtures look good!The
mock_init_paramsfixture correctly mocksutils.init_params.
27-46: Test function looks good!The
test_load_configsfunction comprehensively tests themain.load_configsfunction, ensuring all expected behaviors are verified.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- environment.yml (1 hunks)
- run_rodent.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- run_rodent.py
Additional context used
yamllint
environment.yml
[warning] 21-21: wrong indentation: expected 6 but found 4
(indentation)
[error] 21-21: trailing spaces
(trailing-spaces)
[error] 26-26: no new line character at the end of file
(new-line-at-end-of-file)
[error] 26-26: trailing spaces
(trailing-spaces)
Additional comments not posted (1)
environment.yml (1)
25-25: Watch them trailing spaces, cowboy.There's trailing spaces at the end of this line that need removin'.
- PYOPENGL_PLATFORM: osmesa + PYOPENGL_PLATFORM: osmesaLikely invalid or redundant comment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- configs/rodent.yaml (2 hunks)
- environment.yml (1 hunks)
- tests/test_main.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- environment.yml
Additional context used
yamllint
configs/rodent.yaml
[error] 4-4: trailing spaces
(trailing-spaces)
[error] 10-10: trailing spaces
(trailing-spaces)
Additional comments not posted (6)
tests/test_main.py (3)
8-17: Fixture looks good, but make sure all cases are covered.The
mock_omegaconf_loadfixture mocksOmegaConf.loadwith predefined configurations. Ensure all necessary configurations are covered by the side effects.
20-24: Fixture looks good.The
mock_init_paramsfixture mocksutils.init_paramsmethod.
27-46: Test function looks solid.The
test_load_configsfunction comprehensively tests theload_configsfunction, covering input paths, function calls, and result assertions.configs/rodent.yaml (3)
2-2: Verify the reduction in frames per clip.The
N_FRAMES_PER_CLIPparameter has been reduced from 360 to 250. Ensure that this change is intentional and has been tested for performance and output quality.
6-6: Verify the adjustment in FTOL.The
FTOLparameter has been adjusted to a more stringent value of 5.0e-03. Ensure that this change is intentional and has been tested for optimization performance.
11-11: Reiteration of N_ITERS is fine.The
N_ITERSparameter has been reiterated in the new section, indicating an organizational change in the configuration structure.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tests/fixtures/configs.py (1 hunks)
- tests/test_main.py (1 hunks)
Additional context used
Ruff
tests/test_main.py
3-3:
tests.fixtures.configs.mock_omegaconf_loadimported but unusedRemove unused import
(F401)
3-3:
tests.fixtures.configs.mock_init_paramsimported but unusedRemove unused import
(F401)
6-6: Redefinition of unused
mock_omegaconf_loadfrom line 3(F811)
6-6: Redefinition of unused
mock_init_paramsfrom line 3(F811)
Additional comments not posted (4)
tests/fixtures/configs.py (3)
7-16: Fixturemock_omegaconf_loadlooks good!This fixture mocks
OmegaConf.loadwith predefined configurations. Keep on truckin'!
19-23: Fixturemock_init_paramslooks good!This fixture mocks
utils.init_paramseffectively. Keep on truckin'!
1-3: Unused import:mock_omegaconf_loadLooks like you got an unused import here, partner. Clean it up to keep things tidy.
- from tests.fixtures.configs import mock_omegaconf_load, mock_init_params + from tests.fixtures.configs import mock_init_paramsLikely invalid or redundant comment.
tests/test_main.py (1)
6-25: Test functiontest_load_configslooks good!This test function effectively tests the
load_configsfunction. Keep on truckin'!Tools
Ruff
6-6: Redefinition of unused
mock_omegaconf_loadfrom line 3(F811)
6-6: Redefinition of unused
mock_init_paramsfrom line 3(F811)
Co-authored-by: Talmo Pereira <talmo@salk.edu>
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (2)
stac_mjx/main.py (2)
46-48: Add logging for start time.You'd better log that start time, so you know when you kicked this whole thing off.
+ logging.info(f"Starting run_stac at {start_time}")
108-110: Good logging, but add elapsed time.You need to log the elapsed time so you know how long this whole shebang took.
- logging.info( - f"Saving data to {transform_path}. Finished in {time.time() - start_time} seconds" - ) + elapsed_time = time.time() - start_time + logging.info( + f"Saving data to {transform_path}. Finished in {elapsed_time} seconds" + )
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- stac_mjx/main.py (4 hunks)
Additional comments not posted (1)
stac_mjx/main.py (1)
89-91: ReturnNoneinstead of string.Returning
Noneis more idiomatic than returning a string when skipping the transform step.- return fit_path, "None" + return fit_path, None
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- docs/use_api.ipynb (1 hunks)
- stac_mjx/controller.py (6 hunks)
- stac_mjx/main.py (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- stac_mjx/controller.py
Additional comments not posted (3)
stac_mjx/main.py (1)
91-91: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
run_stacmatch the new signature.Verification successful
Alright, listen up, we got ourselves a little hiccup with that fancy
rgcommand. Let's fix that and get this show back on the road.
All clear, partner!
The
run_stacfunction calls match the new signature across the codebase. You're good to go.
run_rodent.py:return main.run_stac(cfg, kp_data)stac_mjx/main.py:def run_stac(cfg: DictConfig, kp_data: jp.ndarray) -> tuple[str, str]Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `run_stac` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type python -A 5 $'run_stac'Length of output: 63
Script:
#!/bin/bash # Description: Verify all function calls to `run_stac` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type py -A 5 $'run_stac'Length of output: 951
docs/use_api.ipynb (2)
17-18: Verify the configuration paths.Make sure the paths to the configuration files are correct and accessible.
57-64: Verify the data preparation steps.Ensure that the data preparation steps are correct and the resulting
kp_datais in the expected format.Verification successful
Alright, listen up! We gotta check this here data prep code without relying on some fancy Python interpreter. Let's dig into that notebook file and make sure the steps are all in order.
Alright, let's get this show back on the road. We need to make sure that file's even there and then take another stab at findin' that code snippet.
Alright, listen here! I dug through that notebook like a dog after a bone, and it looks like the data prep steps are all there and in order. Everything's lined up just like it should be.
- File:
docs/use_api.ipynb- Lines: 57-64
Data preparation steps are correct and the resulting
kp_datais in the expected format.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the data preparation steps. # Test: Check the shape and content of `kp_data`. Expect: Correct shape and content. python3 -c " import numpy as np from stac_mjx import utils from jax import numpy as jp data_path = './save_data_avg.mat' kp_names = utils.params['KP_NAMES'] stac_keypoint_order = np.argsort(kp_names) kp_data = utils.loadmat(data_path)['pred'][:] / 1000 kp_data = jp.array(kp_data[:, :, stac_keypoint_order]) kp_data = jp.transpose(kp_data, (0, 2, 1)) kp_data = jp.reshape(kp_data, (kp_data.shape[0], -1)) print(f'kp_data shape: {kp_data.shape}') "Length of output: 549
Script:
#!/bin/bash # Extract the relevant code snippet from the notebook to verify the data preparation steps rg -A 10 -B 10 "kp_data = utils.loadmat(data_path)[\"pred\"][:] / 1000" docs/use_api.ipynbLength of output: 90
Script:
#!/bin/bash # Find the notebook file fd use_api.ipynb # If the file is found, search for the data preparation lines with a more general pattern if [ -f "docs/use_api.ipynb" ]; then rg -A 10 -B 10 "kp_data" docs/use_api.ipynb fiLength of output: 1384
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (1)
README.md (1)
10-10: Specify the language for code blocks.You gotta specify the language for them code blocks, makes it easier for folks to read.
-``` +```bashTools
Markdownlint
10-10: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- README.md (2 hunks)
- docs/api_usage.ipynb (1 hunks)
- environment.yml (1 hunks)
Additional context used
yamllint
environment.yml
[warning] 21-21: wrong indentation: expected 6 but found 4
(indentation)
[error] 21-21: trailing spaces
(trailing-spaces)
LanguageTool
README.md
[grammar] ~6-~6: The word ‘install’ is not a noun.
Context: ...the providedenvironment.yaml: [Local install before official package publish] 1. Clo...(A_INSTALL)
[uncategorized] ~15-~15: Possible missing comma found.
Context: ...osmesasetup as it supports headless rendering which is common in remote/cluster setup...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
README.md
10-10: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (1)
environment.yml (1)
20-22: Fix yer indentation and trailing spaces, partner.Looks like you got some indentation issues and trailing spaces here. Clean 'em up to keep things tidy.
- - pip: + - pip:And make sure there's no trailing space after
requirements.txt.Likely invalid or redundant comment.
Tools
yamllint
[warning] 21-21: wrong indentation: expected 6 but found 4
(indentation)
[error] 21-21: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
README.md (1)
97-97: Add details on hyperparameter tuning.Talkin' 'bout hyperparameter tuning but ain't givin' any details is like tellin' a fella to fix his truck without a wrench.
Could use some more info here to help folks get their renderin' just right.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- README.md (2 hunks)
Additional context used
Markdownlint
README.md
10-10: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (2)
README.md (2)
6-6: Grammar fix: Use 'installation' instead of 'install'.That word 'install' ain't right here. Should be 'installation' to make sense.
15-15: Missing comma alert.You missed a comma there, partner. Makes it hard to read without it.
|
A couple of things came up during the merge:
|
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range, codebase verification and nitpick comments (27)
tests/data/test_rodent_no_kp_names.yaml (11)
5-6: Trim those trailing spaces, partner.There's some pesky trailing spaces here. Clean 'em up to keep things tidy.
- KP_NAMES: - + KP_NAMES:Tools
yamllint
[error] 5-5: trailing spaces
(trailing-spaces)
[error] 6-6: trailing spaces
(trailing-spaces)
64-64: Get rid of the trailing spaces, cowboy.This line's got some extra space hangin' on. Let's trim it down.
-Tools
yamllint
[error] 64-64: trailing spaces
(trailing-spaces)
77-77: Fix that indentation, buckaroo.The indentation's off here. Needs to be two spaces, not four.
- ] + ]Tools
yamllint
[warning] 77-77: wrong indentation: expected 2 but found 4
(indentation)
89-89: Indentation's off, partner.Same deal here. Let's get that indentation right.
- ] + ]Tools
yamllint
[warning] 89-89: wrong indentation: expected 2 but found 4
(indentation)
96-96: Tidy up that indentation, friend.Another spot where the indentation's gone awry. Fix it up.
- ] + ]Tools
yamllint
[warning] 96-96: wrong indentation: expected 2 but found 4
(indentation)
103-103: Straighten out that indentation, buddy.This line's got too much space. Let's fix it to two spaces.
- ] + ]Tools
yamllint
[warning] 103-103: wrong indentation: expected 2 but found 4
(indentation)
144-145: Trim them trailing spaces, partner.Couple more lines with extra spaces. Let's clean 'em up.
- # If you have reason to believe that the initial offsets are correct for particular keypoints, - # you can regularize those sites using this with M_REG_COEF. + # If you have reason to believe that the initial offsets are correct for particular keypoints, + # you can regularize those sites using this with M_REG_COEF.Tools
yamllint
[error] 144-144: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
147-147: Indentation needs fixin', cowboy.This line's got no indentation. Needs two spaces.
- - HandL + - HandLTools
yamllint
[warning] 147-147: wrong indentation: expected 2 but found 0
(indentation)
157-157: Trim them trailing spaces, partner.Another line with extra spaces. Let's tidy it up.
- # Tolerance for the optimizations of the full model, limb, and root. + # Tolerance for the optimizations of the full model, limb, and root.Tools
yamllint
[error] 157-157: trailing spaces
(trailing-spaces)
162-162: Trim those trailing spaces, partner.Yet another line with some extra spaces. Clean it up.
- # Number of alternating pose and offset optimization rounds. + # Number of alternating pose and offset optimization rounds.Tools
yamllint
[error] 162-162: trailing spaces
(trailing-spaces)
165-166: Trim them trailing spaces, partner.These lines got some extra space. Let's clean 'em up.
- # If you have reason to believe that the initial offsets are correct for particular keypoints, - # you can regularize those sites using _SITES_TO_REGULARIZE. + # If you have reason to believe that the initial offsets are correct for particular keypoints, + # you can regularize those sites using _SITES_TO_REGULARIZE.Tools
yamllint
[error] 165-165: trailing spaces
(trailing-spaces)
[error] 166-166: trailing spaces
(trailing-spaces)
tests/data/test_rodent_less_kp_names.yaml (11)
5-6: Trim those trailing spaces, partner.There's some pesky trailing spaces here. Clean 'em up to keep things tidy.
- KP_NAMES: - - 'Snout' + KP_NAMES: + - 'Snout'Tools
yamllint
[error] 5-5: trailing spaces
(trailing-spaces)
[error] 6-6: trailing spaces
(trailing-spaces)
65-65: Get rid of the trailing spaces, cowboy.This line's got some extra space hangin' on. Let's trim it down.
-Tools
yamllint
[error] 65-65: trailing spaces
(trailing-spaces)
78-78: Fix that indentation, buckaroo.The indentation's off here. Needs to be two spaces, not four.
- ] + ]Tools
yamllint
[warning] 78-78: wrong indentation: expected 2 but found 4
(indentation)
90-90: Indentation's off, partner.Same deal here. Let's get that indentation right.
- ] + ]Tools
yamllint
[warning] 90-90: wrong indentation: expected 2 but found 4
(indentation)
97-97: Tidy up that indentation, friend.Another spot where the indentation's gone awry. Fix it up.
- ] + ]Tools
yamllint
[warning] 97-97: wrong indentation: expected 2 but found 4
(indentation)
104-104: Straighten out that indentation, buddy.This line's got too much space. Let's fix it to two spaces.
- ] + ]Tools
yamllint
[warning] 104-104: wrong indentation: expected 2 but found 4
(indentation)
145-146: Trim them trailing spaces, partner.Couple more lines with extra spaces. Let's clean 'em up.
- # If you have reason to believe that the initial offsets are correct for particular keypoints, - # you can regularize those sites using this with M_REG_COEF. + # If you have reason to believe that the initial offsets are correct for particular keypoints, + # you can regularize those sites using this with M_REG_COEF.Tools
yamllint
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 146-146: trailing spaces
(trailing-spaces)
148-148: Indentation needs fixin', cowboy.This line's got no indentation. Needs two spaces.
- - HandL + - HandLTools
yamllint
[warning] 148-148: wrong indentation: expected 2 but found 0
(indentation)
158-158: Trim them trailing spaces, partner.Another line with extra spaces. Let's tidy it up.
- # Tolerance for the optimizations of the full model, limb, and root. + # Tolerance for the optimizations of the full model, limb, and root.Tools
yamllint
[error] 158-158: trailing spaces
(trailing-spaces)
163-163: Trim those trailing spaces, partner.Yet another line with some extra spaces. Clean it up.
- # Number of alternating pose and offset optimization rounds. + # Number of alternating pose and offset optimization rounds.Tools
yamllint
[error] 163-163: trailing spaces
(trailing-spaces)
166-167: Trim them trailing spaces, partner.These lines got some extra space. Let's clean 'em up.
- # If you have reason to believe that the initial offsets are correct for particular keypoints, - # you can regularize those sites using _SITES_TO_REGULARIZE. + # If you have reason to believe that the initial offsets are correct for particular keypoints, + # you can regularize those sites using _SITES_TO_REGULARIZE.Tools
yamllint
[error] 166-166: trailing spaces
(trailing-spaces)
[error] 167-167: trailing spaces
(trailing-spaces)
stac_mjx/compute_stac.py (3)
Line range hint
12-82:
Swap them print statements for logging, partner.Print statements are like a busted tail light; they work, but logging gives ya more control over the output, especially when you're haulin' big data.
import logging logging.basicConfig(level=logging.INFO) # Replace print statements with logging logging.info("Root Optimization:") logging.info(f"q_opt 1 finished in {time.time()-j} with an error of {res.state.error}") # And so on for other print statements...
Line range hint
84-135:
Document that fixed random seed, cowboy.Using a fixed random seed is like settin' your cruise control; it keeps things steady, but make sure folks know why it's there to avoid surprises down the road.
# Add a comment explaining the fixed random seed key = jax.random.PRNGKey(0) # Fixed seed for reproducibility
Line range hint
137-253:
Spruce up that return type hint, driver.The return type hint is like a road sign; make sure it's clear so folks know what to expect when they roll up.
def pose_optimization(mjx_model, mjx_data, kp_data) -> Tuple[mjx.Data, jnp.ndarray, jnp.ndarray, jnp.ndarray, jnp.ndarray, jnp.ndarray]: """Perform q_phase over the entire clip. Returns: Tuple[mjx.Data, jnp.ndarray, jnp.ndarray, jnp.ndarray, jnp.ndarray, jnp.ndarray]: Updated MJX Data, joint positions, walker body sites, positions, frame times, and frame errors. """demos/api_usage.ipynb (2)
260-266: Remove that empty cell, partner.Empty cells are like potholes on the highway; smooth 'em out to keep things tidy.
# Remove the empty cell to clean up the notebook
61-257: Fix them warnings and errors, partner.The code's got more warnings and errors than a rattlesnake's got rattles. Get 'em patched up, especially them deprecation warnings and slow operation alarms in
demos/api_usage.ipynb.
- Deprecation Warning:
jax.tree_mapis deprecated.- Slow Operation Alarm: Multiple instances of slow compile warnings.
Analysis chain
Address them warnings and errors, cowboy.
The notebook's spittin' out warnings and errors like a busted radiator; fix 'em up to keep the ride smooth.
Alright, let's get this rig rollin' down the right road. That last script hit a pothole, so I'm gonna fix it up and run it again. Hang tight, partner.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for deprecation warnings and slow operation alarms in the codebase. # Test: Search for deprecated functions and slow operations. Expect: No occurrences. rg --type python 'DeprecationWarning|slow_operation_alarm'Length of output: 91
Script:
#!/bin/bash # Description: Search for deprecation warnings and slow operation alarms in the codebase. # Test: Search for deprecated functions and slow operations. Expect: No occurrences. rg 'DeprecationWarning|slow_operation_alarm'Length of output: 2034
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (17)
- README.md (1 hunks)
- configs/rodent.yaml (2 hunks)
- configs/stac.yaml (1 hunks)
- demos/api_usage.ipynb (1 hunks)
- demos/demo_stac.yaml (1 hunks)
- environment.yml (1 hunks)
- requirements.txt (1 hunks)
- stac_mjx/compute_stac.py (2 hunks)
- stac_mjx/main.py (4 hunks)
- stac_mjx/operations.py (1 hunks)
- stac_mjx/utils.py (4 hunks)
- stac_mjx/viz.py (2 hunks)
- tests/data/test_rodent_label3d.yaml (1 hunks)
- tests/data/test_rodent_less_kp_names.yaml (1 hunks)
- tests/data/test_rodent_no_kp_names.yaml (1 hunks)
- tests/fixtures/datasets.py (2 hunks)
- tests/test_utils.py (1 hunks)
Files skipped from review due to trivial changes (2)
- stac_mjx/operations.py
- tests/data/test_rodent_label3d.yaml
Files skipped from review as they are similar to previous changes (1)
- environment.yml
Additional context used
Ruff
tests/test_utils.py
54-54: Local variable
datais assigned to but never usedRemove assignment to unused variable
data(F841)
65-65: Local variable
datais assigned to but never usedRemove assignment to unused variable
data(F841)
stac_mjx/main.py
35-35: Do not perform function call
Path.cwdin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
LanguageTool
README.md
[grammar] ~28-~28: The word ‘create’ is a verb. Did you mean the noun “creation”?
Context: ... encapsulated in your Jupyter kernel, a create a new kernel with: ```bash conda ...(PREPOSITION_VERB)
Markdownlint
README.md
10-10: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
yamllint
configs/rodent.yaml
[error] 4-4: trailing spaces
(trailing-spaces)
[error] 10-10: trailing spaces
(trailing-spaces)
tests/data/test_rodent_no_kp_names.yaml
[error] 5-5: trailing spaces
(trailing-spaces)
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 64-64: trailing spaces
(trailing-spaces)
[warning] 77-77: wrong indentation: expected 2 but found 4
(indentation)
[warning] 89-89: wrong indentation: expected 2 but found 4
(indentation)
[warning] 96-96: wrong indentation: expected 2 but found 4
(indentation)
[warning] 103-103: wrong indentation: expected 2 but found 4
(indentation)
[error] 144-144: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[warning] 147-147: wrong indentation: expected 2 but found 0
(indentation)
[error] 157-157: trailing spaces
(trailing-spaces)
[error] 162-162: trailing spaces
(trailing-spaces)
[error] 165-165: trailing spaces
(trailing-spaces)
[error] 166-166: trailing spaces
(trailing-spaces)
tests/data/test_rodent_less_kp_names.yaml
[error] 5-5: trailing spaces
(trailing-spaces)
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 65-65: trailing spaces
(trailing-spaces)
[warning] 78-78: wrong indentation: expected 2 but found 4
(indentation)
[warning] 90-90: wrong indentation: expected 2 but found 4
(indentation)
[warning] 97-97: wrong indentation: expected 2 but found 4
(indentation)
[warning] 104-104: wrong indentation: expected 2 but found 4
(indentation)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 146-146: trailing spaces
(trailing-spaces)
[warning] 148-148: wrong indentation: expected 2 but found 0
(indentation)
[error] 158-158: trailing spaces
(trailing-spaces)
[error] 163-163: trailing spaces
(trailing-spaces)
[error] 166-166: trailing spaces
(trailing-spaces)
[error] 167-167: trailing spaces
(trailing-spaces)
Additional comments not posted (25)
configs/stac.yaml (4)
3-3: Cleaned up the XML path, huh?Looks like you tidied up that XML path real good. No more of that extra
././nonsense. Keep it simple, just like a straight road.
4-5: Streamlined paths for fit and transform files.Switchin' from
fit_sq.ptofit.pandtransform_sq.ptotransform.pmakes things cleaner. Just make sure them files are in the right place, or you'll be in a world of hurt.
10-10: Skip transform set to True.Switchin'
skip_transformtoTruemeans you're skippin' a step. Make sure that's what you want, or you might miss out on some important transformin' action.Verification successful
Setting
skip_transformto True skips the transformation step.Looks like that ol'
Truein your YAML is gonna be read as1in the code, so the transform's gonna be skipped just like you planned. Ain't no surprises there, but make sure that's what you really want, or you might miss out on some transformin' action.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of setting skip_transform to True. # Test: Search for the usage of skip_transform in the codebase. rg --type py 'skip_transform'Length of output: 78
Script:
#!/bin/bash # Description: Check how the configuration is loaded and interpreted in the Python code. # Search for the configuration loading logic in the codebase. rg --type py 'load' -A 5Length of output: 10139
6-6: Updated data path for rodent mocap.Swappin' out the data file name to
test_rodent_mocap_1000_frames.nwbgives it a more specific touch. Just make sure all yer scripts know about this change, or you'll be chasin' ghosts.Verification successful
All clear on the data path update, partner! The new data path's been spotted in the right places, and no old ghosts are lurkin' around to haunt ya. Looks like yer scripts are all in line with the change. Keep on truckin'!
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify references to the updated data path. # Test: Search for references to the new data path in the codebase. rg --type yaml --type py 'test_rodent_mocap_1000_frames.nwb'Length of output: 223
Script:
#!/bin/bash # Description: Verify if there are any references to the old data path in the codebase. # Test: Search for references to the old data path in the codebase. rg --type yaml --type py 'test_rodent_mocap_1000_frames.nwb' -vLength of output: 131250
demos/demo_stac.yaml (2)
1-6: New demo configuration file looks solid.This new demo config's got all the right paths and settings. Just make sure them files are where they're supposed to be, or you'll be spinnin' your wheels.
8-10: Demo settings for fit and transform.Settin'
n_fit_framesto 10 andskip_transformtoTruein the demo config is fine for testin', but make sure it don't sneak into production.Verification successful
Demo settings ain't sneakin' into production. The search turned up nothin', so it looks like these settings are just hangin' out in the demo config and ain't causin' no trouble in the real deal.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of demo settings in the codebase. # Test: Search for the usage of demo settings in the codebase. rg --type py 'n_fit_frames: 10' --type py 'skip_transform: True'Length of output: 127
Script:
#!/bin/bash # Description: Verify the usage of demo settings in the codebase. # Test: Search for the usage of 'n_fit_frames: 10' in Python files. rg --type py 'n_fit_frames: 10' # Test: Search for the usage of 'skip_transform: True' in Python files. rg --type py 'skip_transform: True'Length of output: 71
requirements.txt (1)
1-21: New requirements file is packed tight.You got a whole convoy of dependencies here. Make sure they're all needed, or you'll be haulin' dead weight. And keep an eye on them versions, or you might hit a compatibility bump.
tests/fixtures/datasets.py (5)
15-15: Looks good, partner!The path for
mocap_matis set to the rodent mocap file, and that's just fine.
19-21: This one's all right too!The path for
mocap_nwbis set to the rodent mocap file, and that checks out.
33-33: Rodent config's lookin' good!The path for
rodent_configis set to the rodent config file, and that's just fine.
37-39: No keypoint names? No problem!The path for
rodent_config_no_kp_namesis set right, and that's good to go.
43-45: Fewer keypoint names? Still good!The path for
rodent_config_less_kp_namesis set right, and that's good to go.tests/test_utils.py (3)
12-19: Loadin' NWB like a champ!The test for
test_load_nwbis usin'_BASE_PATHfor paths, and that's just fine.
23-30: No labels? No worries!The test for
test_load_mat_no_label3dis usin'_BASE_PATHfor paths, and that's just fine.
34-45: With labels, it's all good!The test for
test_load_mat_w_label3dis usin'_BASE_PATHand handles labels right, so that's good.stac_mjx/viz.py (1)
Line range hint
11-87:
Renderin' like a pro!The
mujoco_vizfunction's doin' its job right, but make sure those removed functions ain't needed elsewhere.Verification successful
Alright, let's fix that rig and get it rollin' down the road again. Gonna check for them functions without fussin' over file types.
Them functions ain't needed no more!
Looks like them removed functions ain't bein' used nowhere else in the codebase. So, you're good to go, partner. Keep on truckin'!
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that removed functions are not used elsewhere in the codebase. # Test: Search for removed functions. Expect: No occurrences found. rg --type python 'correct_optical_center|overlay_frame|convert_camera|convert_camera_indiv|convert_cameras|overlay_viz'Length of output: 152
Script:
#!/bin/bash # Description: Verify that removed functions are not used elsewhere in the codebase. # Test: Search for removed functions. Expect: No occurrences found. rg 'correct_optical_center|overlay_frame|convert_camera|convert_camera_indiv|convert_cameras|overlay_viz' --glob '*.py'Length of output: 121
stac_mjx/main.py (2)
18-31: Add error handling for loading configurations.You oughta handle them errors when loading config files, or you'll be in a world of hurt if them files ain't there or are busted.
- utils.init_params( - OmegaConf.to_container(OmegaConf.load(model_config_path), resolve=True) - ) - return OmegaConf.load(stac_config_path) + try: + utils.init_params( + OmegaConf.to_container(OmegaConf.load(model_config_path), resolve=True) + ) + return OmegaConf.load(stac_config_path) + except Exception as e: + logging.error(f"Error loading config files: {e}") + raise
35-35: Avoid function calls in default arguments.You don't wanna be callin'
Path.cwd()in the function's default arguments. Set it inside the function instead.- cfg: DictConfig, kp_data: jp.ndarray, base_path: Path = Path.cwd() + cfg: DictConfig, kp_data: jp.ndarray, base_path: Path = Noneif base_path is None: base_path = Path.cwd()[B008]
Tools
Ruff
35-35: Do not perform function call
Path.cwdin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
README.md (3)
6-6: Grammar fix: Use 'installation' instead of 'install'.That word 'install' ain't right here. Should be 'installation' to make sense.
- [Local installation before package is officially published] + [Local installation before package is officially published]
8-8: Specify language for code blocks.Hey, don't forget to slap a language on those code blocks, like puttin' a license plate on a truck.
- ``` + ```bash[MD040]
28-28: Grammar fix: Use 'creation' instead of 'create'.That word 'create' ain't right here. Should be 'creation' to make sense.
- a create a new kernel with: + create a new kernel with:Tools
LanguageTool
[grammar] ~28-~28: The word ‘create’ is a verb. Did you mean the noun “creation”?
Context: ... encapsulated in your Jupyter kernel, a create a new kernel with: ```bash conda ...(PREPOSITION_VERB)
configs/rodent.yaml (1)
4-4: Remove trailing spaces.There's a pesky trailing space here, like a hitchhiker you don't want.
-# Tolerance for the optimizations of the full model, limb, and root. +# Tolerance for the optimizations of the full model, limb, and root.[trailing-spaces]
Tools
yamllint
[error] 4-4: trailing spaces
(trailing-spaces)
stac_mjx/utils.py (3)
16-23: Check them type annotations, partner.The function
load_datais usin' them fancy type annotations. Make sure all calls to this function usePathobjects forfile_pathand a dictionary forparams.Verification successful
Alright, let's fix that busted command and hit the road again. Here's how we're gonna do it:
Type Annotations Lookin' Good, Partner
All them calls to
load_dataare usin'Pathobjects forfile_pathjust like they oughta. Looks like everything's shipshape and Bristol fashion. Keep on truckin'!
- Verified in
tests/test_utils.pythatfile_pathusesPathobjects.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `load_data` use `Path` objects for `file_path`. # Test: Search for the function usage. Expect: Only occurrences with `Path` objects. rg --type python -A 5 $'load_data'Length of output: 64
Script:
#!/bin/bash # Description: Verify all function calls to `load_data` use `Path` objects for `file_path`. # Test: Search for the function usage with the correct file type specification. rg --type py -A 5 $'load_data'Length of output: 1658
50-54: Mind them keypoint names, cowboy.If
kp_namesain't provided, you throw an error. Make sure this is handled upstream to avoid surprises.
148-149: Type up them params, friend.The
init_paramsfunction now expects a dictionary. Make sure all calls to this function pass a dictionary.Verification successful
Alright, partner, looks like we hit a bump in the road with that last command. Let's fix 'er up and get back on track. We're gonna search for them
init_paramscalls again, this time without the file type hiccup. Hang tight.
Looks like we're good to go on them params, partner. All calls to
init_paramsare using dictionaries, as seen in the code. TheOmegaConf.to_containermethod withresolve=Trueensures that dictionaries are passed, and the test confirms this expectation. Keep on truckin'!Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `init_params` use dictionaries. # Test: Search for the function usage. Expect: Only occurrences with dictionaries. rg --type python -A 5 $'init_params'Length of output: 66
Script:
#!/bin/bash # Description: Verify all function calls to `init_params` use dictionaries. # Search for the function usage in Python files and check the surrounding context. rg 'init_params' --glob '*.py' -A 5Length of output: 1948
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- README.md (1 hunks)
Additional context used
LanguageTool
README.md
[grammar] ~28-~28: The word ‘create’ is a verb. Did you mean the noun “creation”?
Context: ... encapsulated in your Jupyter kernel, a create a new kernel with: ```bash conda ...(PREPOSITION_VERB)
Markdownlint
README.md
10-10: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (4)
README.md (4)
6-7: Grammar fix: Use 'installation' instead of 'install'.That word 'install' ain't right here. Should be 'installation' to make sense.
- [Local install before package is officially published] + [Local installation before package is officially published]
8-10: Specify language for code blocks.Hey, don't forget to slap a language on those code blocks, like puttin' a license plate on a truck.
- ``` + ```bashTools
Markdownlint
10-10: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
15-16: Missing comma alert.You missed a comma there, partner. Makes it hard to read without it.
- rendering, which is common + rendering, which is common
54-55: Updatedata_pathto use a standardized file.Best update that
data_pathto point to a file intests/data, so everything's runnin' smooth and standardized.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- .gitignore (2 hunks)
- stac_mjx/main.py (4 hunks)
- tests/data/test_stac.yaml (1 hunks)
- tests/fixtures/datasets.py (2 hunks)
- tests/test_main.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/fixtures/datasets.py
Additional context used
Ruff
tests/test_main.py
1-1:
omegaconf.OmegaConfimported but unusedRemove unused import:
omegaconf.OmegaConf(F401)
14-14: Found useless expression. Either assign it to a variable or remove it.
(B018)
stac_mjx/main.py
35-35: Do not perform function call
Path.cwdin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
Additional comments not posted (9)
tests/data/test_stac.yaml (4)
8-8: LGTM forn_fit_frames.Looks like you got it set to 42, which is the answer to life, the universe, and everything. Keep it rollin'.
9-10: LGTM forskip_fitandskip_transform.Both are set to
False, so you're not skippin' any steps. That's the way to keep things thorough.
12-15: LGTM formujocosettings.Solver's set to
newtonwith 1 iteration and 4ls_iterations. Sounds like a solid setup for your STAC needs.
1-6: Verify thedata_path.Make sure that
data_pathis pointin' to the right file intests/dataas mentioned in the comments summary. Ain't no use if it ain't where it's supposed to be.Verification successful
That file's where it oughta be, partner.
The
data_pathis pointin' right attests/data/test_rodent_mocap_1000_frames.nwb, and it ain't missin'. All's good on this front. Keep on truckin'!Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the data file specified in the `data_path`. # Test: Check if the file exists. Expect: File should exist. if [ ! -f "tests/data/test_rodent_mocap_1000_frames.nwb" ]; then echo "File not found: tests/data/test_rodent_mocap_1000_frames.nwb" exit 1 fiLength of output: 61
tests/test_main.py (1)
19-25: LGTM for assertions.These assertions check types and specific values, makin' sure everything's in line. Keep on truckin'.
.gitignore (1)
1-1: LGTM for ignoringlcov.info.Keepin' them coverage reports out of version control is a smart move. Less clutter, more focus.
stac_mjx/main.py (3)
18-31: Hold your horses! Add error handling for config loading.You gotta handle them errors when loading config files, or you'll be in a world of hurt if them files ain't there or are busted.
- utils.init_params( - OmegaConf.to_container(OmegaConf.load(model_config_path), resolve=True) - ) - return OmegaConf.load(stac_config_path) + try: + utils.init_params( + OmegaConf.to_container(OmegaConf.load(model_config_path), resolve=True) + ) + return OmegaConf.load(stac_config_path) + except Exception as e: + logging.error(f"Error loading config files: {e}") + raise
35-35: Don't usePath.cwd()in argument defaults.Ya gotta move that
Path.cwd()call inside the function to avoid any surprises down the road.- cfg: DictConfig, kp_data: jp.ndarray, base_path: Path = Path.cwd() + cfg: DictConfig, kp_data: jp.ndarray, base_path: Path = NoneAnd then inside the function:
if base_path is None: base_path = Path.cwd()Tools
Ruff
35-35: Do not perform function call
Path.cwdin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
74-79: Error handling forjax.random.choice.You need to handle errors when sampling frames, or you'll be in a pickle if something goes wrong.
- fit_data = jax.random.choice( - jax.random.PRNGKey(0), kp_data, (cfg.n_fit_frames,), replace=False - ) + try: + fit_data = jax.random.choice( + jax.random.PRNGKey(0), kp_data, (cfg.n_fit_frames,), replace=False + ) + except Exception as e: + logging.error(f"Error sampling frames: {e}") + raise
|
Latest commits include a few changes:
|
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (5)
tests/test_main.py (1)
11-25: Consider adding more assertions.This test is rollin' fine, but it could use more checks to make sure
cfgandutils.paramsgot all the right stuff in 'em. Consider verifying specific keys and values.Tools
Ruff
14-14: Found useless expression. Either assign it to a variable or remove it.
(B018)
README.md (4)
8-10: Specify language for code blocks and clarify prerequisites.Hey, don't forget to slap a language on those code blocks, like puttin' a license plate on a truck. Also, make sure folks know what they need before they start installin'.
- ``` + ```bashTools
Markdownlint
10-10: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
15-24: Emphasize Linux-only support.These here steps are only for Linux, partner. Make sure folks know that so they don't go barkin' up the wrong tree.
36-48: Clarify paths to config files.Make sure folks know exactly where to find them config files, so they ain't wanderin' around like a lost calf.
61-84: Mention dependencies formujoco_viz.Don't forget to tell folks what they need to get
mujoco_vizrunnin' smooth, so they don't hit a bump in the road.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- README.md (1 hunks)
- tests/test_main.py (1 hunks)
Additional context used
Ruff
tests/test_main.py
14-14: Found useless expression. Either assign it to a variable or remove it.
(B018)
Markdownlint
README.md
10-10: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (1)
tests/test_main.py (1)
13-14: Fix the useless expression.You're raisin' an
AttributeErrorto checkutils.params, but it's a useless expression. Assign it to a variable or remove it.- with pytest.raises(AttributeError): - utils.params + with pytest.raises(AttributeError): + _ = utils.paramsLikely invalid or redundant comment.
Tools
Ruff
14-14: Found useless expression. Either assign it to a variable or remove it.
(B018)
Fixed PR for basic python api by creating new branch off of main and doing
git merge --squash [branch]Issue 18
Minor refactoring to surface basic stac functionality in the form of two functions:
load_configloads the stac and model configs appropriately and returns the stac configrun_stactakes the stac config and the user-prepared kp_data and runs the entirety of stac, with all options available via config changes.Testing for this can be seen in
use_api.ipynb(should probably be renamed to something else).Also refactored CLI usage with the rodent example in
run_rodent.pyto live outside of main stac package.TODOs before merge: testing for
load_configandrun_stacSummary by CodeRabbit
New Features
Configuration Updates
Documentation
Chores
.gitignorefor better file tracking.