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

Fix env None check #3562

Merged
merged 3 commits into from Jun 7, 2018
Merged

Conversation

sjperkins
Copy link
Member

@sjperkins sjperkins commented Jun 5, 2018

I think the if os is None check is invalid, changing it to if env is None.

  • Tests added / passed
  • Passes flake8 dask

@mrocklin
Copy link
Member

mrocklin commented Jun 5, 2018

Hrm, indeed. Thank you for catching this. If you have time it would be useful to establish a simple test to ensure that this kind of thing doesn't happen again.

yaml.dump(b, f)

config = collect([fn1, fn2], env=env)
assert config == expected
Copy link
Member

Choose a reason for hiding this comment

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

Hrm, I think that in order to test the if env is None case we probably need to pass None in, and modify os.environ directly

def test_env_none():
    os.environ['DASK_FOO'] = 'bar'
    try:
        config = collect([])
        assert config['foo'] == 'bar'
    finally:
        del os.environ['DASK_FOO']

@mrocklin
Copy link
Member

mrocklin commented Jun 7, 2018

Looks good to me. Merging. Thanks for identifying and resolving this @sjperkins !

@mrocklin mrocklin merged commit 08f1d03 into dask:master Jun 7, 2018
convexset added a commit to convexset/dask that referenced this pull request Jun 12, 2018
* master:
  [array.linalg.qr] added sfqr (short-and-fat) as a counterpart to tsqr… (dask#3575)
  Drop Dask Array's `learn` module (dask#3580)
  Adds `isneginf` and `isposinf` (dask#3581)
  Update sizeof definitions (dask#3582)
  BUG: do not raise IndexError for identity slice in array.vindex (dask#3559)
  Correct internal dimension of Dask's SVD (dask#3517)
  Fix check in configuration with env=None (dask#3562)
  Dataframe sample method docstring fix (dask#3566)
  Include "region" in key names for da.store (dask#3540)

# Conflicts:
#	dask/array/linalg.py
#	dask/array/tests/test_linalg.py
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.

None yet

2 participants