Switch `FileLoader` to `FileOpener` about data HOT 7 CLOSED

From a purely naming perspective, I think FileOpener is more accurate but FileLoader isn't wildly misleading either.

I had a look at numpy.load, json.load, pickle.load, these seem to either read or parse through files that have already been opened and return some structured data. In that sense, FileLoader definitely behaves differently relative to other modules. Renaming it is likely better.

Perhaps we can rename it, but still leave FileLoader functional with a deprecation warning? I would imagine most people who started using IterDataPipe in PyTorch Core would be using FileLoader and it would be a BC-breaking change.

from data.

I had a look at numpy.load, json.load, pickle.load, these seem to either read or parse through files that have already been opened and return some structured data. In that sense, FileLoader definitely behaves differently that other modules. Renaming it is likely better.

Thanks for digging into it. You are right. And, the most concerning part is the functional API since we are currently using load_file_... to for each FileLoader. I don't want users to complain the inconsistent behavior across libs.

Perhaps we can rename it, but still leave FileLoader functional with deprecation warning? I would imagine most people who started using IterDataPipe in PyTorch Core would be using FileLoader and it would be a BC-breaking change.

Thank you for pointing out. If we plan to move, we should definitely add deprecation warning at least before official release.

from data.

Also want to gather some insights from domains since this is going to be BC breaking.
cc: @pmeier @Nayef211

from data.

Also want to gather some insights from domains since this is going to be BC breaking. cc: @pmeier @Nayef211

I agree that FileLoader sounds misleading and that FileOpener would be a better name for what the datapipe is actually doing. I also think adding a deprecation warning is a good idea, so that users have time to migrate to the new datapipe.

from data.

+1 for FileOpener.

Not sure about the deprecation warning though. I mean torchdata is not even "released" yet and is also clearly labeled as prototype. Of course you can go for it, but that also ups the maintenance burden. For torchvision it is fine to ping me on PRs that break BC so I can land a fix quickly.

from data.

I agree with @pmeier mainly because we didn't mention these DataPipes and functionalities in PyTorch release. And, based on the prototyping policy, we should be able to switch name directly and prevent users to keep using a deprecated feature or name at prototyping phase.

Another way as I mentioned is to add a deprecation warning before our official release. Then, cleaning up during our branch cut. It would add more burden to us maintaining the repo.

I would prefer option 1.

from data.

I agree that based on the prototyping policy we should be able to rename as we wish. I think the policy is very clear for things that exist in TorchData. Do you think that policy is clear to users for things that are in PyTorch Core? If so, then we can rename it without deprecation.

from data.