Skip to content

Don't evaluate config in skip condition#10753

Merged
phofl merged 5 commits intodask:mainfrom
phofl:env_var
Jan 8, 2024
Merged

Don't evaluate config in skip condition#10753
phofl merged 5 commits intodask:mainfrom
phofl:env_var

Conversation

@phofl
Copy link
Copy Markdown
Collaborator

@phofl phofl commented Jan 5, 2024

  • Closes #xxxx
  • Tests added / passed
  • Passes pre-commit run --all-files

@phofl phofl marked this pull request as draft January 5, 2024 22:35
@phofl phofl marked this pull request as ready for review January 6, 2024 14:49
Copy link
Copy Markdown
Member

@hendrikmakait hendrikmakait left a comment

Choose a reason for hiding this comment

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

TIL: You can't evaluate the config in pytest.marks. LGTM, just a small PEP8 nit.

except ImportError:
ArrowNotImplementedError = RuntimeError # some unrelated error to make pytest pass

dask_expr_enabled = dd._dask_expr_enabled()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Use capital letters to highlight constant

Suggested change
dask_expr_enabled = dd._dask_expr_enabled()
DASK_EXPR_ENABLED = dd._dask_expr_enabled()

from dask.utils import M
from dask.utils_test import _check_warning, hlg_layer

dask_expr_enabled = dd._dask_expr_enabled()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
dask_expr_enabled = dd._dask_expr_enabled()
DASK_EXPR_ENABLED = dd._dask_expr_enabled()

@phofl phofl merged commit e353f87 into dask:main Jan 8, 2024
@phofl phofl deleted the env_var branch January 8, 2024 10:43
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.

2 participants