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

Filters (may) only work with CSV sources that have defined columns #161

Open
ptgolden opened this issue Dec 4, 2024 · 1 comment
Open
Assignees

Comments

@ptgolden
Copy link
Member

ptgolden commented Dec 4, 2024

This is the code that designates how data should be filtered in the transform:

# Filter columns
filtered_columns = [column_filter.column for column_filter in self.filters]
all_columns = []
if self.columns:
all_columns = [next(iter(column)) if isinstance(column, Dict) else column for column in self.columns]
if self.header == HeaderMode.none and not self.columns:
raise ValueError(
"there is no header and columns have not been supplied\n"
"configure the 'columns' field or set header to the 0-based"
"index in which it appears in the file, or set this value to"
"'infer'"
)
for column in filtered_columns:
if column not in all_columns:
raise (ValueError(f"Filter column {column} not in column list"))

I may be following it incorrectly, but if self.columns is not set, then all_columns is always equal to [], and a ValueError is raised for any filter. self.columns is not set when:

  • there is a non-csv source
  • the header mode is set to HeaderMode.infer and columns is not provided

It may not be possible to tell in advance what fields may exist in a read record if:

  • there is a JSON(L) source and a field is not given in required_properties
  • there is a CSV reader, the header mode is set to HeaderMode.infer, and columns is not provided

The solution here is either to:

  • allow filter fields that are not explicitly expected in the configuration (via columns in the CSV reader or required_attributes in the JSON(L) reader)
  • require fields that will be filtered on to be explicitly stated
@ptgolden
Copy link
Member Author

ptgolden commented Dec 4, 2024

For now, I will raise a value error /only/ in the case where self.columns is set.

@ptgolden ptgolden self-assigned this Dec 5, 2024
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

No branches or pull requests

1 participant