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

Utilize echopype working directory #897

Closed
lsetiawan opened this issue Dec 8, 2022 · 2 comments
Closed

Utilize echopype working directory #897

lsetiawan opened this issue Dec 8, 2022 · 2 comments
Assignees
Milestone

Comments

@lsetiawan
Copy link
Member

Overview

After the acceptance of the idea for an echopype working directory, the team have decided to do the following after PR #896 is merged:

  • move all the temp directories and temp zarr files into this hidden directory, instead of the current temp_echopype_output directory
  • move all the testing framework and files into this hidden directory

Originally posted by @leewujung in #896 (comment)

@leewujung
Copy link
Member

A note that I was just looking at what combine_echodata does -- seems that if no destination path is given, the combined file will get saved at temp_echopype_output/combined_echodatas.zarr (see here).

I have two complaints:
1 . echodataS doesn't make sense to me -- how about just combined _echodata.zarr?
2. currently in io.validate_output_path, the way docstring is written it is not clear that whatever in source_file is ignored if save_path is a full path (including filename) and not just a path to a folder:

if is_dir:
check_file_permissions(sanitized_path)
out_path = os.path.join(save_path, Path(source_file).stem + file_ext)
else:
if isinstance(sanitized_path, Path):
check_file_permissions(sanitized_path.parent)
final_path = sanitized_path
out_path = str(final_path.parent.joinpath(final_path.stem + file_ext).absolute())
else:
path_dir = fsspec.get_mapper(os.path.dirname(save_path), **output_storage_options)
check_file_permissions(path_dir)
final_path = Path(save_path)
out_path = save_path
if final_path.suffix != file_ext:
logger.warning(
"Mismatch between specified engine and save_path found; forcing output format to engine." # noqa
)

@lsetiawan : what do you think about making these changes as you work on this issue to use the hidden default directory? For 2. it could just a docstring change for clarification, but it feels a little bizarre to have source_file and save_path partially overlap only sometimes. Thoughts? (I am fine with keeping it as is to not disturb anything major, but thought we should discuss regardless.)

@leewujung
Copy link
Member

This is addressed in #954.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Echopype Mar 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

2 participants