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

Consistent handling of paths #93

Closed
Peter9192 opened this issue Jun 1, 2021 · 4 comments · Fixed by #143
Closed

Consistent handling of paths #93

Peter9192 opened this issue Jun 1, 2021 · 4 comments · Fixed by #143

Comments

@Peter9192
Copy link
Collaborator

Peter9192 commented Jun 1, 2021

The ewatercycle user interface uses string formats to get path inputs. It would be helpful if we internally default to using standard Path representations as well. #82 implements a pathparser that converts input strings to pathlib objects. It would be nice use this throughout the package.

It might also be nice to add some custom __str__ methods so that the user never gets to see the internal representation.

@sverhoeven sverhoeven modified the milestone: Tech paper Jun 23, 2021
@Peter9192 Peter9192 changed the title Use dateparser and pathparser everywhere Consistent handling of paths Jun 25, 2021
@Peter9192 Peter9192 added this to the Tech paper milestone Jun 25, 2021
@sverhoeven
Copy link
Member

sverhoeven commented Jun 30, 2021

By fixing #113 we no longer leak paths to user with the public API.

Converting strings to Path objects is now duplicated in each model. We can make generic function in ewatercycle.util to perform this conversion so expanduser() and resolve() are called always. Something like

def make_absolute(p: str) -> Path:
  return Path(p).expanduser().resolve()

@Peter9192
Copy link
Collaborator Author

Yep, like that 👍

Don't we still leak paths when the user tries to inspect the ParameterSet or Forcing objects?

@Peter9192
Copy link
Collaborator Author

I made a start with something like that in #107

@Peter9192
Copy link
Collaborator Author

ewatercycle.parameter_sets.default.make_absolute() can be reused in ewatercycle.forcing.default. Maybe move to util.

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 a pull request may close this issue.

2 participants