Skip to content

Conversation

@edgarcosta
Copy link
Member

  • The title is concise and informative.
  • I have created tests covering the changes.

@github-actions
Copy link

github-actions bot commented Oct 13, 2025

Documentation preview for this PR (built with commit 8885ac3; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@edgarcosta edgarcosta marked this pull request as ready for review October 13, 2025 19:06
@orlitzky
Copy link
Contributor

Ok, it looks good to me, pending whatever those CI jobs are waiting for.

@user202729
Copy link
Contributor

@sagemath/core please approve workflows.

(I guess this shows being added to sagemath/triage doesn't help with getting workflows approved at all.)

"""
Append the .sobj extension to a filename if it doesn't already have it.
"""
s = os.fspath(s) # Convert path-like objects to strings
Copy link
Contributor

Choose a reason for hiding this comment

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

This line should be useless

return

filename = filename[0]
filename = os.fspath(filename) # Convert path-like objects to strings
Copy link
Contributor

Choose a reason for hiding this comment

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

the comment doesn't add anything meaningful to the code and is pure noise, delete.

This function needs doctest.

make sure if filename is originally a Path then the branch below with fetching URL is never executed.

@edgarcosta edgarcosta force-pushed the copilot/fix-save-path-object-issue branch from 73d5d53 to 8885ac3 Compare October 14, 2025 21:31
@edgarcosta edgarcosta requested a review from user202729 October 15, 2025 20:38
Copy link
Contributor

@fchapoton fchapoton left a comment

Choose a reason for hiding this comment

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

ok

vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 18, 2025
sagemathgh-41037: Enable save/load to accept Path objects
    
- [x] The title is concise and informative.
- [x] I have created tests covering the changes.
    
URL: sagemath#41037
Reported by: Edgar Costa
Reviewer(s): Edgar Costa, Frédéric Chapoton, Michael Orlitzky, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 19, 2025
sagemathgh-41037: Enable save/load to accept Path objects
    
- [x] The title is concise and informative.
- [x] I have created tests covering the changes.
    
URL: sagemath#41037
Reported by: Edgar Costa
Reviewer(s): Edgar Costa, Frédéric Chapoton, Michael Orlitzky, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 20, 2025
sagemathgh-41037: Enable save/load to accept Path objects
    
- [x] The title is concise and informative.
- [x] I have created tests covering the changes.
    
URL: sagemath#41037
Reported by: Edgar Costa
Reviewer(s): Edgar Costa, Frédéric Chapoton, Michael Orlitzky, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 26, 2025
sagemathgh-41037: Enable save/load to accept Path objects
    
- [x] The title is concise and informative.
- [x] I have created tests covering the changes.
    
URL: sagemath#41037
Reported by: Edgar Costa
Reviewer(s): Edgar Costa, Frédéric Chapoton, Michael Orlitzky, user202729
@vbraun vbraun merged commit b2b6663 into sagemath:develop Oct 27, 2025
25 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.

5 participants