Skip to content

Conversation

edgarcosta
Copy link
Member

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants