Add file utils

[collindutter]

Adds file utils to fill void left by removal of file loading logic in Loaders.

Specifically relevant in doc examples that use load_collection.

[collindutter]

Wondering if load_files should just return a list so it's easier to pass into load_collection? Don't think we can load the files concurrently if we change that though.

[dylanholmes]

You could have two functions, something like load_file_collection and load_file_list. Then you can identify individual files in the response if you need to, otherwise the load_file_list will be more convenient. The implementation of load_file_list could just call load_file_collection (obviously).

Another more heavy handed alternative could be to allow passing a dict[str, bytes] into load_collection in addition to list[bytes] for each of the relevant loaders. A pro to this option is that you could even use the original key passed in to the input, so the client needed to map from a file name to the file contents, they wouldn't need to do two lookups.

Currently:

sources = [ 'foo.txt' ]
content_by_filename_hash = utils.load_files(sources)
artifacts_by_content_hash = loader.load_collection(list(files_by_filename.values()))

foo_content = content_by_filename_hash[hash_from_str('foo.txt')]
foo_artifact = artifacts_by_content_hash[loader.to_key(foo_content)]

With suggestion:

sources = [ 'foo.txt' ]
content_by_filename_hash = utils.load_files(sources)
artifacts_by_filename_hash = loader.load_collection(content_by_filename_hash)

# One less lookup if `load_collection` reuses keys when passed a dict
foo_artifact = artifacts_by_filename_hash[hash_from_str('foo.txt')]

If the input is the same shape as the output of load_collection, then it'd make it easier to chain together loaders in general, however I can't think of a use case besides this one.

---

Another alternative that I'm not necessarily advocating for due to increase in scope, but we could make loaders composable and re-introduce file loader.

[dylanholmes]

Nice work!

Approving since the core use case is solved, but added some alternative suggestions for you to consider at your discretion.

[dylanholmes]

You could have two functions, something like load_file_collection and load_file_list. Then you can identify individual files in the response if you need to, otherwise the load_file_list will be more convenient. The implementation of load_file_list could just call load_file_collection (obviously).

Another more heavy handed alternative could be to allow passing a dict[str, bytes] into load_collection in addition to list[bytes] for each of the relevant loaders. A pro to this option is that you could even use the original key passed in to the input, so the client needed to map from a file name to the file contents, they wouldn't need to do two lookups.

Currently:

sources = [ 'foo.txt' ]
content_by_filename_hash = utils.load_files(sources)
artifacts_by_content_hash = loader.load_collection(list(files_by_filename.values()))

foo_content = content_by_filename_hash[hash_from_str('foo.txt')]
foo_artifact = artifacts_by_content_hash[loader.to_key(foo_content)]

With suggestion:

sources = [ 'foo.txt' ]
content_by_filename_hash = utils.load_files(sources)
artifacts_by_filename_hash = loader.load_collection(content_by_filename_hash)

# One less lookup if `load_collection` reuses keys when passed a dict
foo_artifact = artifacts_by_filename_hash[hash_from_str('foo.txt')]

If the input is the same shape as the output of load_collection, then it'd make it easier to chain together loaders in general, however I can't think of a use case besides this one.

---

Another alternative that I'm not necessarily advocating for due to increase in scope, but we could make loaders composable and re-introduce file loader.

[collindutter]

Also just learned a new python syntax, maybe these changes aren't needed?

with open("tests/resources/cities.csv", "r") as cities, open("tests/resources/addresses.csv", "r") as addresses:
    CsvLoader().load_collection([cities.read(), addresses.read()])

[vasinov]

Can we move this into a method parameter with a default value?

[collindutter]

I can, but do we still want this PR even with the discussed syntax above? You can see it live here

[vasinov]

Ha! Does it parallelize open?

[collindutter]

I'm struggling to find a first-party resource that discusses the concurrency. But all mentions of the syntax online seem to suggest it is.

Even still, it might be nice to provide these utils as a slightly less painful migration path for users.

[collindutter]

Ended up adding file utils to docs too

[collindutter]

Previously used test.txt is generated by other tests and is not a reliable source.

[collindutter]

Bump on this question

[vasinov]

Arg doc for futures_executor is missing.

[collindutter]

I can't figure out why tests are failing on 3.9, and 3.11. Some sort of file race condition? The file should be there...