Filenames now must allow references to Path

[ChristianBeilschmidt]

• I agree to follow the project's code of conduct.
• I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

---

According to affirmation in #227 I changed all file paths to be implementing AsRef<Path>.
This is based on @lnicola's idea and is similar to https://doc.rust-lang.org/std/fs/struct.File.html#method.open.

This introduced generics on the methods but I guess less code-breaking since you can still input &strs.

[lnicola]

In general, my preference is to split the non-generic parts of those functions like in https://doc.rust-lang.org/src/std/fs.rs.html#928-930. But I suppose you could argue that most of these functions are relatively small, so I don't have a strong opinion on this.

PS: I haven't looked too close otherwise.

[ChristianBeilschmidt]

In general, my preference is to split the non-generic parts of those functions like in https://doc.rust-lang.org/src/std/fs.rs.html#928-930. But I suppose you could argue that most of these functions are relatively small, so I don't have a strong opinion on this.

PS: I haven't looked too close otherwise.

What is the reason or advantage for doing this? I guess it could be an improvement if we call the method internally lots of times and save some as_ref calls but I doubt that this is the case.

[lnicola]

Non-generic code is compiled in the crate where it's defined. Generic code is compiled in the crate where it's used, possibly multiple times (either with different generic arguments or if it's used from multiple functions and they end up in different CGUs). This is similar to #[inline], except that it doesn't get inlined.

This is fine for something like Vec<T> where you want the best codegen possible (even there only the size of T matters if it has a trivial Drop), but a large function with a path.as_ref() at the beginning is the worst case possible because it makes the whole thing generic.

But as I said, it probably doesn't matter too much here.

[ChristianBeilschmidt]

Okay, I didn't have that in mind. But still, you wouldn't change it for this PR?

[lnicola]

Sorry for taking so long to review this. I think at least Dataset::open_ex, Dataset::create_copy and Driver::create_with_band_type_with_options are large enough to warrant splitting.

But if I was doing this, I'd probably split everything except Dataset::open and Driver::create_with_band_type.

[lnicola]

Suggested change

	Self::_create_copy(&self, driver, filename.as_ref(), options)
	self._create_copy(driver, filename.as_ref(), options)

[lnicola]

Suggested change

	Self::_delete(&self, filename.as_ref())
	self._delete(filename.as_ref())

[lnicola]

Suggested change

	Self::_rename(&self, new_filename.as_ref(), old_filename.as_ref())
	self._rename(new_filename.as_ref(), old_filename.as_ref())

[lnicola]

Suggested change

	let file_name_c = _path_to_c_string(&file_name)?;
	let file_name_c = _path_to_c_string(file_name)?;

[lnicola]

Suggested change

	let file_name_c = _path_to_c_string(&file_name)?;
	let file_name_c = _path_to_c_string(file_name)?;

[lnicola]

Suggested change

	Self::_create_with_band_type_with_options::<T>(
	&self,
	self._create_with_band_type_with_options::<T>(

[lnicola]

Some clippy lints, but it looks all right. I'll merge it tomorrow unless anyone else objects.

[jdroenner]

Great! thanks fr reviewing @lnicola . i can create a new release in the next days :)

[ChristianBeilschmidt]

Some clippy lints, but it looks all right. I'll merge it tomorrow unless anyone else objects.

I had an old Rust version installed since we use a toolchain file for Geo Engine 😄. But I've fixed them in
4d4e040 .

[lnicola]

"Tomorrow"

bors r+

[bors]

Build succeeded:

• ci ubuntu-20.04
• ci ubuntu-20.04 (--all-features)
• clippy