Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Move settings.py to habitat_sim.utils #1832

Merged
merged 6 commits into from
Sep 7, 2022
Merged

Conversation

jaraujo98
Copy link
Collaborator

Motivation and Context

One task I have to do frequently is to start a “default” simulator (mostly for writing custom viewers). The only option I am aware of is to use the code in habitat-sim/examples/settings.py to get a default configuration. Having to rely on this file has been a pain point. I have to either run my scripts from the examples folder or the sim folder (this leads to inconsistent imports: on some files the statement is from settings import ..., on others it is from examples.settings import ...). I can also play around with sys.path or PYTHONPATH, but this not a viable solution since it can clash with other packages that also have folders named examples or files named settings.

This PR moves the settings.py file into habitat_sim.utils, thus making its functionality unambiguously available everywhere. I kept a file named settings.py inside examples that simply imports the original settings file to make sure existing scripts that rely on it don't break.

How Has This Been Tested

Tests (some of which rely on this file) pass. The existing examples also continue working.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Aug 17, 2022
@jaraujo98 jaraujo98 requested a review from aclegg3 August 17, 2022 18:43
Copy link
Contributor

@aclegg3 aclegg3 left a comment

Choose a reason for hiding this comment

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

Great to see the tests no longer pulling in examples. Mostly I suggest we extend both default_settings and make_cfg() in examples and move some things there.

src_python/habitat_sim/utils/settings.py Outdated Show resolved Hide resolved
src_python/habitat_sim/utils/settings.py Outdated Show resolved Hide resolved
src_python/habitat_sim/utils/settings.py Outdated Show resolved Hide resolved
src_python/habitat_sim/utils/settings.py Show resolved Hide resolved
src_python/habitat_sim/utils/settings.py Outdated Show resolved Hide resolved
examples/demo_runner.py Outdated Show resolved Hide resolved
Copy link
Contributor

@aclegg3 aclegg3 left a comment

Choose a reason for hiding this comment

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

LGTM

@jaraujo98 jaraujo98 merged commit 4d553b2 into main Sep 7, 2022
@jaraujo98 jaraujo98 deleted the default-settings branch September 7, 2022 16:38
@jaraujo98 jaraujo98 mentioned this pull request Sep 15, 2022
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants