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

get_project 'root' keyword fails silently #779

Closed
rskye144 opened this issue Jun 21, 2022 · 5 comments
Closed

get_project 'root' keyword fails silently #779

rskye144 opened this issue Jun 21, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@rskye144
Copy link
Contributor

rskye144 commented Jun 21, 2022

Description

When the get_project function is called with a project root that does not exist, it silently defaults to a project in the current directory.

To reproduce

import signac
project1 = signac.init_project()
project2 = signac.get_project("some/directory/that/does/not/exist")  # this should raise to prevent user error, rather than searching upward from a nonexistent directory
assert project1 == project2  # this is true but we shouldn't get this far

Error also occurs because paths with ~ are not expanded, producing an unexpected error; @bdice suggests could consider adding:

if "~" in root:
    warnings.warn("The character '~' is in the provided root path. Did you mean to call os.path.expanduser(root)?", UserWarning)

Error output

No error is raised (which is wrong, an error/warning should be raised)

@mikemhenry mikemhenry added the bug Something isn't working label Jun 21, 2022
@mikemhenry
Copy link
Collaborator

@rskye144 👋 thanks for the bug report 😍

Are you interested in contributing a PR to fix this issue? We can provide some help/guidance if you are interested!

@bdice
Copy link
Member

bdice commented Jun 22, 2022

I think the right place to put this is below this line:

root = os.getcwd()

I think it should say:

def load_config(root=None, local=False):
    """Load configuration, searching upward from a root path."""
    if root is None:
        root = os.getcwd()
    if not os.path.isdir(root):
        raise ValueError(f"The root directory '{root}' does not exist.")
    # rest of the function

@rskye144
Copy link
Contributor Author

I can give a PR a try! It will be my first time doing that, so any advice is appreciated.

@bdice
Copy link
Member

bdice commented Aug 1, 2022

One key piece that I didn't notice earlier: this bug occurs fails for relative paths contained inside the current directory, when the current directory contains a signac project. Getting a nonexistent absolute path like signac.get_project("/a/fake/path") fails with a LookupError.

bdice added a commit that referenced this issue Aug 10, 2022
* first try at changing config.py

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* changed pytest location to tests

* changed error catch to get_project()

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* added contribution requirements and fixed double formatting

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update changelog.

* Revert whitespace changes.

* Remove unused import.

* Update contributors.yaml

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Bradley Dice <[email protected]>
@vyasr
Copy link
Contributor

vyasr commented Nov 2, 2022

Resolved by #792

@vyasr vyasr closed this as completed Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants