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

Make path consistent #143

Merged
merged 33 commits into from
Jul 8, 2021
Merged

Make path consistent #143

merged 33 commits into from
Jul 8, 2021

Conversation

SarahAlidoost
Copy link
Contributor

@SarahAlidoost SarahAlidoost commented Jul 6, 2021

closes #93
after merging this, close PR #107

@SarahAlidoost SarahAlidoost marked this pull request as ready for review July 7, 2021 09:31
Copy link
Member

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

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

I searched for usage of expanduser and resolve and it is now only used in ewatercycle/util.py:to_absolute_path, in ewatercycle/config/_config_object.py:load_from_file and in ewatercycle/observation/grdc.py:get_grdc_data . Can Config.load_from_file and the grdc be changed to also use to_absolute_path()?

I searched for Path constructor and mostly shows up in util.py.

This PR has a lot of smart replacements. Nice work.

See inline questions and suggestions.

ewatercycle/forcing/__init__.py Show resolved Hide resolved
ewatercycle/forcing/_default.py Outdated Show resolved Hide resolved
tests/test_util.py Outdated Show resolved Hide resolved
tests/test_util.py Show resolved Hide resolved
ewatercycle/models/wflow.py Show resolved Hide resolved
ewatercycle/models/marrmot.py Show resolved Hide resolved
ewatercycle/models/lisflood.py Show resolved Hide resolved
ewatercycle/models/abstract.py Show resolved Hide resolved
ewatercycle/util.py Show resolved Hide resolved
Copy link
Member

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

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

Look goods, thanks for the additional tests.

@sonarcloud
Copy link

sonarcloud bot commented Jul 8, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

98.7% 98.7% Coverage
1.4% 1.4% Duplication

@SarahAlidoost
Copy link
Contributor Author

Look goods, thanks for the additional tests.

Thanks for reviewing, nice comments, and pair-programming.

@SarahAlidoost SarahAlidoost merged commit 673a069 into master Jul 8, 2021
@SarahAlidoost SarahAlidoost deleted the make_path_consistent branch July 8, 2021 14:15
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.

Consistent handling of paths
3 participants