Skip to content

DirFileSystem.mv should call the underlying fs #1582

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

Closed
wants to merge 1 commit into from

Conversation

nilshamerlinck
Copy link
Contributor

@nilshamerlinck nilshamerlinck commented Apr 19, 2024

  • The issue was discussed here: Error on mv using DirFileSystem with fs=SMBFileSystem #1335
  • Also taking the opportunity to rename mv_file into mv where needed (as mv_file is not defined in the interface of AbstractFileSystem, while mv is) but keeping mv_file as a copy of mv for retro-compatibility in case someone is using it

self._join(path1),
self._join(path2),
**kwargs,
)

mv_file = mv
Copy link
Member

Choose a reason for hiding this comment

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

Does anything call mv_file? In most cases, the _file means it operates on a single file at a time, whereas the one without does possible lists and glob expansion. Perhaps we need both?

Copy link
Member

@martindurant martindurant left a comment

Choose a reason for hiding this comment

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

Since mv_file is not in the spec, and by convention should not mean the same as mv, I would consider removing it wherever it appears.

@nilshamerlinck
Copy link
Contributor Author

ok @martindurant, done in #1585

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.

2 participants