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

Error handling: when saving a datastack with relative paths fails #1630

Merged

Conversation

emilyanndavis
Copy link
Member

@emilyanndavis emilyanndavis commented Sep 17, 2024

Description

Fixes #1608 by logging and rendering an error message on failure to save a datastack with relative paths (e.g., when os.path.relpath raises a ValueError on Windows).

relpath-failure_2024-09-17

Checklist

  • Updated HISTORY.rst and link to any relevant issue (if these changes are user-facing)
    - [ ] Updated the user's guide (if needed)
  • Tested the Workbench UI on macOS (by forcing datastack.build_parameter_set to raise a ValueError)
  • Tested the Workbench UI on Windows (thanks, @davemfish!)

Copy link
Contributor

@davemfish davemfish left a comment

Choose a reason for hiding this comment

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

@emilyanndavis This is working exactly as described for me on Windows!

It's great to have this new error-handling in place. I had a few minor comments about try/except/else, but in general everything looks very thorough good.

src/natcap/invest/datastack.py Outdated Show resolved Hide resolved
src/natcap/invest/datastack.py Outdated Show resolved Hide resolved
src/natcap/invest/ui_server.py Outdated Show resolved Hide resolved
workbench/src/renderer/components/SetupTab/index.jsx Outdated Show resolved Hide resolved
Copy link
Member

@dcdenu4 dcdenu4 left a comment

Choose a reason for hiding this comment

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

Alright, thanks @emilyanndavis! I did a quick lookover and Dave thought it was good to go as well.

@dcdenu4 dcdenu4 disabled auto-merge October 14, 2024 12:48
@dcdenu4 dcdenu4 removed the request for review from davemfish October 14, 2024 12:51
@dcdenu4 dcdenu4 disabled auto-merge October 14, 2024 12:53
@dcdenu4 dcdenu4 self-assigned this Oct 14, 2024
@dcdenu4 dcdenu4 dismissed davemfish’s stale review October 14, 2024 12:56

I think we're good, Dave gave the thumbs up but can't access this PR right now.

@dcdenu4 dcdenu4 merged commit 232ca0e into natcap:main Oct 14, 2024
26 of 29 checks passed
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.

Handle saving datastack with relative paths, across drives/volumes
3 participants