Skip to content

Removes usage of flatdict#4585

Open
ewirbel wants to merge 9 commits intoisaac-sim:developfrom
ewirbel:ewirbel/flatdict_removal
Open

Removes usage of flatdict#4585
ewirbel wants to merge 9 commits intoisaac-sim:developfrom
ewirbel:ewirbel/flatdict_removal

Conversation

@ewirbel
Copy link
Copy Markdown

@ewirbel ewirbel commented Feb 10, 2026

Description

This change removes the flatdict dependency entirely. flatdict is a very old dependency that has recently broken compatibility with modern install practices, and was used in exactly one circumstance: to build settings from a nested dictionary.

This change removes the flatdict dependency and implements the dictionary flattening manually instead.

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

- removing flatdict dependency from install requirements
- creating a new method that does the dict flattening to avoid having to keep that dep on
@github-actions github-actions bot added documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team labels Feb 10, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 10, 2026

Greptile Overview

Greptile Summary

This PR removes the flatdict dependency across the project, updates docs and install requirements accordingly, and replaces the prior FlatDict(..., delimiter=".") usage with a local to_flat_dict() helper used when loading render preset .kit files into carb settings. Tests were updated to validate preset expansion using the new helper.

Confidence Score: 4/5

  • Mostly safe to merge once the to_flat_dict recursion bug is fixed.
  • The dependency removal is straightforward and tests were updated, but to_flat_dict() currently ignores the caller-provided delimiter on recursive calls, which can produce incorrect flattened keys if a non-default delimiter is used in the future or elsewhere.
  • source/isaaclab/isaaclab/sim/simulation_context.py

Important Files Changed

Filename Overview
CONTRIBUTORS.md Adds a new contributor entry (no functional impact).
docs/conf.py Removes flatdict from autodoc_mock_imports to reflect dependency removal.
source/isaaclab/docs/CHANGELOG.rst Adds a 0.54.4 changelog entry documenting flatdict dependency removal.
source/isaaclab/isaaclab/sim/simulation_context.py Replaces flatdict usage with new to_flat_dict() helper; found a bug where recursive calls ignore the passed delimiter.
source/isaaclab/setup.py Removes flatdict from INSTALL_REQUIRES so package no longer depends on it.
source/isaaclab/test/sim/test_simulation_render_config.py Updates tests to use to_flat_dict() instead of flatdict; no additional issues spotted.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant SC as SimulationContext
    participant TOML as toml.load
    participant FLAT as to_flat_dict
    participant Carb as carb.settings

    Caller->>SC: _apply_render_settings_from_cfg()
    SC->>TOML: load(rendering_modes/<mode>.kit)
    TOML-->>SC: nested dict
    SC->>FLAT: to_flat_dict(preset_dict, delimiter=".")
    FLAT-->>SC: flat key/value pairs
    loop each preset entry
        SC->>Carb: set_setting("/"+key.replace(".", "/"), value)
    end
    SC-->>Caller: render settings applied
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

6 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@kellyguo11 kellyguo11 changed the title Ewirbel/flatdict removal Removes usage of flatdict Feb 11, 2026
@kellyguo11 kellyguo11 changed the base branch from main to develop February 11, 2026 21:25
Copy link
Copy Markdown
Contributor

@matthewtrepte matthewtrepte left a comment

Choose a reason for hiding this comment

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

looks good to me

@ewirbel ewirbel changed the base branch from develop to main February 12, 2026 17:44
@ewirbel ewirbel force-pushed the ewirbel/flatdict_removal branch from 11f9b94 to fd6a7f5 Compare February 12, 2026 17:45
@kellyguo11 kellyguo11 changed the base branch from main to develop February 12, 2026 22:13
@kellyguo11
Copy link
Copy Markdown
Contributor

Thanks a lot for this change! I retargeted the PR to the develop branch instead since that's where we've moved our recent development to. Could you do an update to the latest develop?

Please also update the version in the extension.toml file under isaaclab to match with the new version in the CHANGELOG.

Signed-off-by: Kelly Guo <kellyg@nvidia.com>
@kellyguo11
Copy link
Copy Markdown
Contributor

hmm looks like there's a failing test in test_dict.py

🚀 Running /workspace/isaaclab/source/isaaclab/test/utils/test_dict.py independently...

============================= test session starts ==============================
collected 6 items

source/isaaclab/test/utils/test_dict.py .....F                           [100%]

=================================== FAILURES ===================================
______________________________ test_to_flat_dict _______________________________
source/isaaclab/test/utils/test_dict.py:118: in test_to_flat_dict
    assert len(flattened_dict) == len(expected_keys)
E   AssertionError: assert 8 == 6
E    +  where 8 = len({'a': 1, 'b': 2, 'c.d': 3, 'c.e': 4, ...})
E    +  and   6 = len(['a', 'b', 'c.d', 'c.e', 'c.f.g', 'c.f.h'])
- generated xml file: /workspace/isaaclab/tests/test-reports-test_dict.py.xml --
=========================== short test summary info ============================

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants