Reorg into src directory #61
Conversation
WalkthroughRebases project layout to a src/ package: updates CI, linters, typing config, docs, gitignore, pyproject packaging/dependencies, and moves CLI entry points under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (22)
.flake8(1 hunks).github/workflows/run-tests.yml(3 hunks).gitignore(1 hunks).mypy.ini(1 hunks)docs/conf.py(1 hunks)docs/developer_guide_backplanes.rst(2 hunks)docs/developer_guide_best_practices.rst(1 hunks)docs/developer_guide_building_docs.rst(1 hunks)docs/developer_guide_configuration.rst(1 hunks)docs/developer_guide_extending.rst(4 hunks)docs/developer_guide_introduction.rst(1 hunks)docs/user_guide_backplanes.rst(3 hunks)docs/user_guide_navigation.rst(1 hunks)pyproject.toml(3 hunks)src/main/nav_backplane_viewer.py(2 hunks)src/main/nav_backplanes.py(2 hunks)src/main/nav_backplanes_cloud_tasks.py(1 hunks)src/main/nav_create_simulated_image.py(2 hunks)src/main/nav_offset.py(1 hunks)src/main/nav_offset_cloud_tasks.py(3 hunks)src/nav/_version.py(1 hunks)src/util/report_profile.py(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-24T19:48:51.676Z
Learnt from: rfrenchseti
Repo: SETI/rms-nav PR: 48
File: main/nav_main_offset.py:167-171
Timestamp: 2025-11-24T19:48:51.676Z
Learning: In the nav_main_offset.py file, it is normal and expected for the nav_default_config.yaml file to be missing. The FileNotFoundError suppression when attempting to load this default config is intentional behavior, not an error condition that needs logging.
Applied to files:
.gitignoredocs/user_guide_navigation.rst
📚 Learning: 2025-11-24T02:08:32.443Z
Learnt from: rfrenchseti
Repo: SETI/rms-nav PR: 48
File: nav/config_files/config_90_backplanes.yaml:29-47
Timestamp: 2025-11-24T02:08:32.443Z
Learning: In nav/config_files/config_90_backplanes.yaml, the `phase_angle` method is intentionally shared between body and ring backplanes. While other ring backplane methods use ring-specific names (e.g., `ring_emission_angle`, `ring_angular_resolution`), `phase_angle` works for both bodies and rings without needing a `ring_phase_angle` variant.
Applied to files:
docs/developer_guide_backplanes.rst
📚 Learning: 2025-11-10T22:24:08.880Z
Learnt from: rfrenchseti
Repo: SETI/rms-nav PR: 14
File: nav/obs/__init__.py:36-44
Timestamp: 2025-11-10T22:24:08.880Z
Learning: In nav/obs/__init__.py, the __all__ list should maintain semantic grouping rather than alphabetical ordering: generic classes (Obs, ObsSnapshot, ObsSnapshotInst) first, followed by specific instrument classes (ObsCassiniISS, etc.), then helper functions (inst_names, inst_name_to_obs_class).
Applied to files:
docs/developer_guide_extending.rst
📚 Learning: 2025-11-10T22:26:37.929Z
Learnt from: rfrenchseti
Repo: SETI/rms-nav PR: 14
File: nav/support/correlate_old.py:83-88
Timestamp: 2025-11-10T22:26:37.929Z
Learning: The file nav/support/correlate_old.py contains legacy code and should be excluded from code reviews.
Applied to files:
.flake8
🧬 Code graph analysis (1)
src/main/nav_backplanes.py (2)
src/nav/config/config.py (1)
backplanes(218-222)src/backplanes/backplanes.py (1)
generate_backplanes_image_files(19-179)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test nav (ubuntu-latest, 3.13)
- GitHub Check: Test nav (ubuntu-latest, 3.10)
- GitHub Check: Test nav (ubuntu-latest, 3.11)
- GitHub Check: Test nav (ubuntu-latest, 3.12)
🔇 Additional comments (29)
.gitignore (1)
168-168: LGTM! Entry aligns with the reorganization and expected behavior.Adding
nav_default_config.yamlto.gitignoreis appropriate given that this file is expected to be missing or generated locally (as per the FileNotFoundError suppression pattern). This complements the reorganization intosrc/.Optional suggestion: Consider adding a brief comment above this entry (e.g.,
# Navigation default config (generated/environment-specific)) to clarify its purpose, especially if other developers may wonder why this file is ignored..mypy.ini (1)
4-4: The experiments directory path and exclusion are correctly configured.The
src/experimentsdirectory exists with the reorganized codebase, and the exclude path is properly formatted for mypy. The configuration aligns with the repo's strict type checking setup (strict = True) while explicitly opting experiments out of type checking, which appears intentional.docs/developer_guide_configuration.rst (1)
1-9: LGTM! Documentation updated for src/ reorganization.The path reference has been correctly updated to reflect the new directory structure.
src/main/nav_backplanes_cloud_tasks.py (2)
17-20: LGTM! Standard sys.path bootstrapping pattern.This allows the CLI to be run directly from the source tree. The E402 flake8 ignore in .flake8 for src/main/*.py files confirms this pattern is intentional.
22-26: Verify import path consistency across CLI scripts.The import path for
backplanes.backplaneswas changed whilenav.*imports remain prefixed. This suggestsbackplanesandnavare sibling directories undersrc/. Ensure all CLI scripts follow the same import pattern.#!/bin/bash # Verify import patterns in CLI scripts after sys.path bootstrap # Check all CLI scripts for import patterns echo "=== Checking import patterns in src/main/*.py files ===" for file in src/main/*.py; do echo "File: $file" # Look for sys.path manipulation if grep -q "sys.path.insert" "$file"; then echo " Has sys.path bootstrap: YES" # Check imports after the sys.path line awk '/sys.path.insert/,0' "$file" | grep -E "^from (nav|backplanes|experiments|util)" | head -5 else echo " Has sys.path bootstrap: NO" fi echo "" done echo "=== Verify directory structure under src/ ===" fd -t d -d 1 . src/docs/developer_guide_introduction.rst (1)
1-3: LGTM! Formatting update.Title underline adjusted for consistency with other documentation files.
docs/developer_guide_best_practices.rst (1)
1-3: LGTM! Formatting update.Title underline adjusted for consistency.
src/util/report_profile.py (1)
3-7: LGTM! Added return type annotation.Good practice to add type hints. The formatting adjustments are minor improvements.
docs/developer_guide_building_docs.rst (1)
1-3: LGTM! Formatting update.Title underline adjusted for consistency.
.flake8 (1)
4-14: LGTM! Flake8 configuration updated for src/ layout.All paths correctly updated to reflect the new directory structure. The E402 ignore for
src/main/*.pyis appropriate for CLI scripts that use sys.path bootstrapping to enable running from the source tree.Based on learnings, the exclusion of
src/nav/support/correlate_old.py(legacy code) is maintained.src/main/nav_create_simulated_image.py (2)
4-4: LGTM! Import added for sys.path bootstrapping.The
osimport is needed for the path manipulation logic added below.
41-44: LGTM! Standard sys.path bootstrapping pattern.This enables running the CLI directly from the source tree by adding the
src/directory to the module search path. The pattern is consistent with other CLI scripts in this PR.src/main/nav_backplanes.py (2)
17-20: LGTM: Source tree execution bootstrap.The sys.path manipulation correctly enables running the CLI directly from the source tree, consistent with other scripts in this PR.
30-30: The import is correct and consistent with the repository's module structure.backplanesis intentionally a top-level package atsrc/backplanes/, not under thenavnamespace. The sys.path manipulation at line 20-21 is documented and designed to make CLI tools executable from the source tree—this is standard practice and not fragile. The architectural separation is by design, not accidental.Likely an incorrect or invalid review comment.
src/main/nav_backplane_viewer.py (1)
36-39: LGTM: Consistent source tree bootstrap.The sys.path manipulation correctly enables source-tree execution and is consistent with the pattern used across CLI scripts in this PR.
.github/workflows/run-tests.yml (2)
19-19: LGTM: GitHub Actions version upgrades.Upgrading to actions/checkout@v6 and actions/setup-python@v6 brings the workflow to the latest stable versions.
Also applies to: 22-22, 55-55, 58-58
32-32: LGTM: Tool targets updated for src/ layout.The flake8 and mypy targets correctly reflect the reorganized project structure.
Also applies to: 36-36
src/main/nav_offset_cloud_tasks.py (2)
18-21: LGTM: Source tree bootstrap.The sys.path manipulation is consistent with other CLI scripts and enables source-tree execution.
128-154: LGTM: Well-designed entry point refactoring.The refactoring separates the async implementation (
async_main()) from the synchronous entry point wrapper (main()). This is excellent design that properly supports setuptools console script entry points, which require synchronous functions.docs/user_guide_navigation.rst (1)
104-104: LGTM: Documentation updated for new layout.The path correctly reflects the reorganized directory structure.
src/nav/_version.py (1)
31-34: Version metadata update (auto-generated).This file is auto-generated by setuptools-scm and the version/commit changes are expected. No review needed.
docs/conf.py (1)
10-10: LGTM: Sphinx configuration updated for src/ layout.The path adjustment ensures Sphinx autodoc can properly import modules from the reorganized structure.
src/main/nav_offset.py (1)
21-24: LGTM: Source tree bootstrap.The sys.path manipulation is consistent with other CLI scripts and correctly enables source-tree execution.
docs/developer_guide_backplanes.rst (1)
1-79: LGTM! Documentation paths updated consistently.All path references have been correctly updated from
nav/*tosrc/nav/*to reflect the new source directory structure.docs/user_guide_backplanes.rst (1)
1-111: LGTM! User guide paths updated consistently.All path references in examples and configuration descriptions have been correctly updated to reflect the new
src/nav/structure.docs/developer_guide_extending.rst (1)
1-98: LGTM! Developer guide paths updated consistently.All path references for extending the system have been correctly updated to the new
src/nav/structure across all sections (Dataset, Instrument, Navigation Model, and Navigation Technique).pyproject.toml (3)
57-60: LGTM! Pytest configuration is correct.The pytest
pythonpathconfiguration correctly points to thesrcdirectory, which is the standard approach for src-layout projects.
71-71: LGTM! Version file path updated correctly.The version file path has been correctly updated to reflect the new
src/nav/_version.pylocation.
14-26: All newly added dependencies are actively used and necessary for the project:
- PyQt6: Used extensively for GUI components across multiple UI modules
- rms-cloud_tasks: Used in cloud processing scripts for distributed task execution
- scipy: Used for scientific computing operations (image processing, signal analysis, interpolation)
- ruamel.yaml: Used for YAML-based configuration file handling
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #61 +/- ##
==========================================
+ Coverage 34.63% 36.98% +2.35%
==========================================
Files 63 57 -6
Lines 4678 4350 -328
Branches 616 564 -52
==========================================
- Hits 1620 1609 -11
+ Misses 2958 2641 -317
Partials 100 100 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test nav (ubuntu-latest, 3.12)
- GitHub Check: Test nav (ubuntu-latest, 3.10)
- GitHub Check: Test nav (ubuntu-latest, 3.11)
- GitHub Check: Test nav (ubuntu-latest, 3.13)
🔇 Additional comments (3)
pyproject.toml (3)
57-60: LGTM!The pytest configuration with
pythonpath = ["src"]is correct for the src-layout and ensures tests can properly import packages during test execution.
64-64: LGTM!The package discovery configuration has been correctly fixed to use
where = ["src"]as previously recommended. This properly configures setuptools to find packages inside the src directory.
71-71: LGTM!The version file path has been correctly updated to
src/nav/_version.pyto align with the new src-layout.
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
nav,backplanes,main,experiements, andutilinto newsrcdirectory.Summary by CodeRabbit
New Features
Chores
Tests / Tooling
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.