Update documentation around `VersionedS3Path` and `PureVersionedS3Path`

[nlangellier]

This PR includes the documentation to go with the new VersionedS3Path and PureVersionedS3Path classes introduced in #139.

Additionally:

1. The VersionedS3Path.__repr__ method was moved to the PureVersionedS3Path class so that both classes now have the correct representation string.
2. The pathlib.PurePath.joinpath method was overridden in PureS3Path to account for the new possibilities created with the addition of the new VersionedS3Path and PureVersionedS3Path classes.

[nlangellier]

@liormizr this is ready for review.

[liormizr]

Hi @nlangellier

First thing sorry for the confusion with the last PR I forgot that we have more work with the doc's..

About the doc's: In docs/interface.rst the doc's are design to look like the standard library pathlib documentation, So I recommend changing the order so all VersionedS3Path/PureVersionedS3Path changes will be under their section.

In S3Path/PureS3Path methods remove the VersionedS3Path notes Instead under VersionedS3Path/PureVersionedS3Path specify the changes for each method that we changed the behavior / API.

About the S3Path.joinpath method: In the normal Pathlib design we are note changing the class under the hood I don't want to change the pathlib behavior and I don't want to touch S3Path class :-).

One suggestion that I can think of in this case is to let version_id to be by default to be None And to check version_id in the _absolute_path_validation method In this case you can always use VersionedS3Path and only in methods that require absolute path we verify that we have version details as well.

What do you think?

[nlangellier]

@liormizr Thank you for your review!

As for interface.rst:

I thought I had mostly followed the pathlib doc structure (with the misplacement of PureVersionedS3Path definition that I corrected). When I look at the pathib docs I see the following structure:

• Pure paths
  • List of definitions of pure paths
  • Definition of PurePath
  • Definition of PurePosixPath
  • Definition of PureWindowsPath
  • Operators
  • Accessing individual parts
  • Methods and properties
  • Definition of PurePath.drive
  • ...
• Concrete paths
  • List of definitions of concrete paths
  • Definition of Path
  • Definition of PosixPath
  • Definition of WindowsPath
  • Methods
  • Definition of Path.cwd
  • ...

I tried to follow this structure and adapt to your existing docs by doing:

• Concrete paths
  • List of definitions of concrete paths
  • Definition of S3Path
  • Definition of VersionedS3Path
  • Methods
  • Definition of S3Path.stat
  • Definition of VersionedS3Path.stat
  • Definition of S3path.exists
  • ...
• Pure paths
  • List of definitions of pure paths
  • Definition of PureS3Path
  • Definition of PureVersionedS3Path
  • PureS3Path has a similar behavior to PurePosixPath, except for the below changes:
  • PureVersionedS3Path has a similar behavior to PureS3Path, except for the below changes:

But it sounds like you want to separate S3Path & PureS3Path docs entirely from VersionedS3Path & PureVersionedS3Path docs, which feels like a deviation from the pathlib docs structure. Happy to make this separation if you want, but just a little confused. Can you clarify please?

As for joinpath:

I moved PureS3Path.joinpath to PureVersionedS3Path since you don't want to modify S3Path or PureS3Path but I worry this still leaves one unresolved case:

>>> s3_path = S3Path("foo/bar")
>>> versioned_s3_path = VersionedS3Path("bat/baz", version_id="<version_id>")
>>> s3_path.joinpath("bap", versioned_s3_path)
S3Path('foo/bar/bap/bat/baz')

In the case where the final argument of S3Path.joinpath is of type VersionedS3Path, wouldn't we want the resulting object to also be of type VersionedS3Path? If yes, then I would offer that we could define PureS3Path.joinpath as:

def joinpath(self, *args):

    if (len(args) > 0) and isinstance(args[-1], PureVersionedS3Path):
        return VersionedS3Path(self, *args, version_id=args[-1].version_id)

    return super().joinpath(*args)

That way, the behavior of PureS3Path.joinpath is only changed in the narrow case where the final argument is of type PureVersionedS3Path. Also I didn't fully understand your idea about version_id defaulting to None, but from what I did understand about it, I'm not confident it will resolve the narrow case I posed above. Was there something about that idea that I missed that would resolve this case? I'm also happy to not override PureS3Path.joinpath at all and leave this edge case unresolved if it is a hard requirement that we leave PureS3Path untouched. WDYT?

[nlangellier]

As an aside, I hope you find our exchanges engaging as opposed to annoying. I'm always seeking to find the best solutions possible subject to the constraints of the repo owner. But if you do find our exchanges annoying, please let me know so that we can find a more constructive path forward :)

[liormizr]

I'm not annoyed :-) I love this kind of brain storming If I'm not annoy you, then all is good

I'll update you tomorrow with my feedback

[nlangellier]

Sounds great! 👍

[nlangellier]

@liormizr are you still planning to provide your thoughts on my most recent comments?

[liormizr]

@nlangellier Sorry for the delay We will go forward and adjust over time if needed

[nlangellier]

We will go forward and adjust over time if needed

Sounds like a good plan 👍

Nice work and thank you for your work

Thanks! And thank you for all your feedback.