Skip to content

Comments

Fallback for storing blend file#46

Merged
yuxuanzhuang merged 2 commits intomainfrom
save_fall_back
Mar 30, 2025
Merged

Fallback for storing blend file#46
yuxuanzhuang merged 2 commits intomainfrom
save_fall_back

Conversation

@yuxuanzhuang
Copy link
Owner

Fixes #44

Changes made in this Pull Request:

  • add fallback to store .blend file to the current directory with unique name

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

@codecov
Copy link

codecov bot commented Mar 30, 2025

Codecov Report

Attention: Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.

Project coverage is 75.11%. Comparing base (c1f91f9) to head (559830b).
Report is 1 commits behind head on main.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tylerjereddy
Copy link
Contributor

Oof mock-testing... if this is causing too much trouble for you--just wanted to mention that I think you are probably right that the real problem was just the missing slash / in the original uuid patch as you noted at: #39 (comment)

I think this may still be useful, but perhaps not something you should spend too much time on given that it is pretty likely that the postdoc had an issue writing to a path that was something like /tmp739a6ad3-c1c2-4b13-a915-390af5aefd8a instead of /tmp/739a6ad3-c1c2-4b13-a915-390af5aefd8a. So maybe "nice to have" just in case, but perhaps not a huge priority in hindsight.

@yuxuanzhuang
Copy link
Owner Author

Enable this test (and not os.remove('test-ggmolvis-uuid.blend')) will lead to an issue during Blender clean up.

  File "ggmolvis/__init__.py", line 80, in cleanup_function
    frame_change_pre.remove(update_frame)
ValueError: list.remove(x): x not in list

Maybe these's a better way to test it?

@tylerjereddy
Copy link
Contributor

Maybe, but also not offended if you want to just close my issue :)

@yuxuanzhuang
Copy link
Owner Author

Oof mock-testing... if this is causing too much trouble for you--just wanted to mention that I think you are probably right that the real problem was just the missing slash / in the original uuid patch as you noted at: #39 (comment)

I think this may still be useful, but perhaps not something you should spend too much time on given that it is pretty likely that the postdoc had an issue writing to a path that was something like /tmp739a6ad3-c1c2-4b13-a915-390af5aefd8a instead of /tmp/739a6ad3-c1c2-4b13-a915-390af5aefd8a. So maybe "nice to have" just in case, but perhaps not a huge priority in hindsight.

Yes, I’ll leave it as is for now :)

@yuxuanzhuang yuxuanzhuang merged commit 95c63c3 into main Mar 30, 2025
9 of 14 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.

ENH: ggmolvis.blend permissions fallback

2 participants