Skip to content

--Scene Dataset Config paths glob/wildcard handling#1152

Merged
jturner65 merged 5 commits intofacebookresearch:masterfrom
jturner65:MM_PathPatternHandling
Mar 25, 2021
Merged

--Scene Dataset Config paths glob/wildcard handling#1152
jturner65 merged 5 commits intofacebookresearch:masterfrom
jturner65:MM_PathPatternHandling

Conversation

@jturner65
Copy link
Copy Markdown
Contributor

Motivation and Context

This PR adds the ability to provide glob wildcards in the scene dataset configuration for the paths to specific configs, enabling wild-card-based searches and loads for configs that may be spread across many directories, such as per-stage paths that exist in Mp3d and HM3D datsets. See the newly added config file data/test_assets/dataset_tests/dataset_1/test_dataset_1.scene_dataset_config.json for usage examples.

How Has This Been Tested

C++ and python tests pass

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 Mar 23, 2021
Copy link
Copy Markdown
Contributor

@mosra mosra left a comment

Choose a reason for hiding this comment

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

The glob() API usage is simpler than I expected, nice.

Comment thread src/esp/io/io.h Outdated
#ifndef ESP_IO_IO_H_
#define ESP_IO_IO_H_

#include <glob.h>
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.

Can be in the cpp instead :)

Comment thread src/esp/io/io.h
/**
* @brief This function will perform globbing on a passed path + pattern,
* returning all the files and directories that match the pattern.
* @param pattern The pattern to match
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.

Maybe mention what is allowed in the pattern? As in, it's just wildcards and not full regular expressions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is there a way in the docs to put an external html link? (I was thinking of linking wiki :D )

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.

It's a standard Markdown, so [title](url) works. Or just put an URL there directly.

Copy link
Copy Markdown
Contributor

@Skylion007 Skylion007 left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me.

@jturner65 jturner65 merged commit d6d9536 into facebookresearch:master Mar 25, 2021
@jturner65 jturner65 deleted the MM_PathPatternHandling branch March 25, 2021 19:53
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.

4 participants