Split out FilenameProvider, rename DirectoryProvider to PathProvider

[jwlodek]

Creating this as a draft PR first to get some feedback before I apply fixes to the filewriters to account for this change.

Addresses #214

[jwlodek]

Before I continue, I'd like to hear if you have any suggestions. @coretl @mrakitin @DiamondJoseph

[coretl]

I can think of 2 usecases for us:

• For testing we would like all the files for a given detector to be placed in a predictable tmp_path. I guess we could either use a different StaticPathProvider and AutoIncrementingFilenameProvider for each detector, or we could pass down device_name to all FilenameProviders and allow AutoIncrementingFilenameProvider to use that in its format string.
• In production we will probably make a PathProvider that doesn't use a FilenameProvider at all, just generates the names according to DLS conventions

So feel free to delete all the prefix and suffix code that you wouldn't use yourself.

[mrakitin]

I'll review once rebased :)

[jwlodek]

@mrakitin @danielballan @coretl I think this is ready for review and potentially merge

[mrakitin]

Looks great to me! Proposed some non-blocking suggestions, most of them are optional.

[mrakitin]

I have some fixes from yesterday's and today's work at HEX in jwlodek#1. Please merge, then we can rebase.

[mrakitin]

Not sure if it's a leftover from my rebasing...
Is it all now implemented in https://github.com/bluesky/ophyd-async/blob/main/src/ophyd_async/epics/areadetector/writers/general_hdffile.py?

[mrakitin]

OK, this is passing now! Codecov is failing, but I don't think it should be a blocker.
A couple of questions to @coretl, @evalott100, and @abbiemery are below.

[mrakitin]

I guess we'll also need https://github.com/bluesky/ophyd-async/blob/main/src/ophyd_async/epics/areadetector/writers/general_hdffile.py, not this file, right?

[mrakitin]

A comment #245 (review) was intended to be a part of my review. Sorry for posting it separately.

[DiamondJoseph]

I think we should make this explicitly a Callable with no input args: if you want to use a uuid generator that takes arguments, you should construct a partial and pass that.

This then is just a CallableFileNameProvider, as it could take any partial not just a UUID one.

The case of needing the current state to calculate the FileName isn't covered here: it's always taking the same uuid_call_args with every call.

[DiamondJoseph]

Or make call_func: Callable[[P], str], call_args: P where P = ParamSpec("P")

[DiamondJoseph]

Suggested change

	@pytest.fixture
	def static_path_provider_factory(tmp_path: Path):
	def create_static_dir_provider_given_fp(fp: FilenameProvider):
	return StaticPathProvider(fp, tmp_path)
	return create_static_dir_provider_given_fp
	@pytest.fixture
	def static_path_provider(
	static_path_provider_factory: callable,
	static_filename_provider: FilenameProvider,
	):
	return static_path_provider_factory(static_filename_provider)
	@pytest.fixture
	def static_path_provider(tmp_path: Path, static_filename_provider: StaticFilenameProvider):
	return StaticPathProvider(static_filename_provider, tmp_path)

Do you need this factory function elsewhere? It doesn't seem to be required.