Skip to content

Fix object import - #1738

Open
linsword13 wants to merge 1 commit into
Ramble-Project:developfrom
linsword13:ramble-import
Open

Fix object import#1738
linsword13 wants to merge 1 commit into
Ramble-Project:developfrom
linsword13:ramble-import

Conversation

@linsword13

Copy link
Copy Markdown
Collaborator

Previously things like import ... as is not supported. Change to use importlib to support such standard access patterns.

Also unify the previous ObjectNamespace and RepositoryNamespace, into a single RambleNamespace.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors module lazy-loading in ramble.repository by replacing ObjectNamespace and RepositoryNamespace with a unified RambleNamespace class, and switching from import to importlib.import_module. It also adds corresponding unit tests. The feedback highlights an issue where catching all ImportError exceptions and raising AttributeError masks nested import errors within the imported modules. It is recommended to only raise AttributeError if the ImportError matches the target submodule name, allowing other import errors to propagate.

Comment thread lib/ramble/ramble/repository.py
@ramble-pr-bot

ramble-pr-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown

Ramble Performance Test Metrics

Results produced with commit: 660da65

Test Name Outcome Duration (s) Most Recent Run (s) Last 5 Avg (s)
test_analyze_large_file passed 2.0638 2.0776 (2d42cdb) 2.0633
test_large_template_expansion passed 1.1828 1.1994 (2d42cdb) 1.1989
test_many_experiments passed 23.5939 23.2386 (2d42cdb) 23.6468
test_many_objects_defaults passed 12.3030 12.1959 (2d42cdb) 12.5012
test_matrix_filter_perf passed 1.0461 1.0590 (2d42cdb) 1.0757

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.42857% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.10%. Comparing base (b311f41) to head (9afcd34).

Files with missing lines Patch % Lines
lib/ramble/ramble/repository.py 80.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1738      +/-   ##
===========================================
+ Coverage    94.07%   94.10%   +0.02%     
===========================================
  Files          370      370              
  Lines        37424    37432       +8     
===========================================
+ Hits         35207    35225      +18     
+ Misses        2217     2207      -10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rfbgo
rfbgo requested a review from douglasjacobsen August 24, 2026 15:49
@douglasjacobsen

Copy link
Copy Markdown
Collaborator

@linsword13 LGTM. Some merge conflicts need resolving though

@linsword13

Copy link
Copy Markdown
Collaborator Author

@linsword13 LGTM. Some merge conflicts need resolving though

Thanks! Should be resolved now.

@rfbgo rfbgo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dcoapp recheck

Previously things like `import ... as` is not supported. Change to use `importlib` to support such standard access patterns.

Also unify the previous `ObjectNamespace` and `RepositoryNamespace`, into a single `RambleNamespace`.

Signed-off-by: Lin Guo <linsword13@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants