Skip to content

Commit

Permalink
Move load_options out from the File object (#1721)
Browse files Browse the repository at this point in the history
# Description
## What is the current behavior?
load_options exposed to the user in File location/type added in the
following PR - #1540. Since
it's a derived property and not something the user should set.

Concerns
load_options shouldn't be exposed to the end users in the File class.
load_options is specific to load_file() and should not be part of
File/Databases.

closes: #1719


## What is the new behavior?
We added a getter/setter property on the File class such that the
load_option interface is not exposed to the end user.

## Does this introduce a breaking change?
Yes

### Checklist
- [ ] Created tests which fail without the change (if possible)
- [ ] Extended the README / documentation, if necessary
  • Loading branch information
utkarsharma2 authored Feb 8, 2023
1 parent c212f4d commit 61a371d
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 19 deletions.
40 changes: 27 additions & 13 deletions python-sdk/src/astro/files/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ class File(LoggingMixin, Dataset):
normalize_config: dict | None = None
is_dataframe: bool = False
is_bytes: bool = False
load_options: list[LoadOptions] | None = None

uri: str = field(init=False)
extra: dict | None = field(init=False, factory=dict)
Expand All @@ -44,6 +43,22 @@ class File(LoggingMixin, Dataset):
"conn_id",
)

@property
def load_options(self):
"""
Getter of all the load_options. load_options is a container with for the custom option passed by user for a
third-party integrations like pandas, azure etc.
"""
return getattr(self, "_load_options", [])

@load_options.setter
def load_options(self, value: LoadOptionsList):
"""
Setter of all the load_options. load_options is a container with for the custom option passed by user for a
third-party integrations like pandas, azure etc.
"""
self._load_options = value

@property
def location(self) -> BaseFileLocation:
return create_file_location(self.path, self.conn_id, self.load_options_list)
Expand Down Expand Up @@ -250,18 +265,17 @@ def resolve_file_path_pattern(
:param normalize_config: parameters in dict format of pandas json_normalize() function
"""
location = create_file_location(path_pattern, conn_id)

files = [
File(
path=path,
conn_id=conn_id,
filetype=filetype,
normalize_config=normalize_config,
load_options=load_options,
)
for path in location.paths
if not path.endswith("/")
]
files = []
for path in location.paths:
if not path.endswith("/"):
file = File(
path=path,
conn_id=conn_id,
filetype=filetype,
normalize_config=normalize_config,
)
file.load_options = load_options
files.append(file)
if len(files) == 0:
raise FileNotFoundError(f"File(s) not found for path/pattern '{path_pattern}'")

Expand Down
10 changes: 5 additions & 5 deletions python-sdk/tests/files/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,11 @@ def test_file_object_picks_load_options(file_type, file_location):
location_path, location_expected_class = file_location.values()
file = File(
path=location_path + f".{type_name}",
load_options=[
PandasLoadOptions(delimiter="$"),
SnowflakeLoadOptions(copy_options={}),
WASBLocationLoadOptions(storage_account="some_account"),
],
)
file.load_options = [
PandasLoadOptions(delimiter="$"),
SnowflakeLoadOptions(copy_options={}),
WASBLocationLoadOptions(storage_account="some_account"),
]
assert type(file.type.load_options) is type_expected_class
assert file.location.load_options is location_expected_class
2 changes: 1 addition & 1 deletion python-sdk/tests/files/type/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def test_repr():
file = File(path="astro", conn_id="local", filetype=FileType.CSV)
assert file.__repr__() == (
"File(path='astro', conn_id='local', filetype=<FileType.CSV: 'csv'>, "
"normalize_config=None, is_dataframe=False, is_bytes=False, load_options=None, "
"normalize_config=None, is_dataframe=False, is_bytes=False, "
"uri='astro+file://local@/astro?filetype=csv', extra={})"
)

Expand Down

0 comments on commit 61a371d

Please sign in to comment.