Path copy_append and join_right keep hash_overwrite

[albertz]

Example code:

path = tk.Path("foo", hash_overwrite="X")
path2 = path.join_right("bar")

path2.hash_overwrite == "X"  # ?
sis_hash(path) == sis_hash(path2)  # ?

I assume this is not intended? At least I was very confused about this.

[JackTemaki]

Do you mean both evaluate to true? This is then indeed not what I would have expected.

[albertz]

Yes both evaluate to true.

[critias]

You right, that isn't intended. In both cases the whole Path object is copied, including hash_overwrite.
I added a PR to change the path given by hash_overwrite as well.

[albertz]

The PR should fix this.

I wonder a bit, it feels like Path.hash_overwrite is not so well tested, maybe not so often used? Because this issue here and also #94 look like having Path("foo") vs Path("bar", hash_overwrite="foo") would often behave different and even potentially lead to different hashes (when join_right is used, or when the order is relevant, etc).

[critias]

I only used it very rarely and don't know how often it is used in other places.

[JackTemaki]

I only ever used for including external data. For the new way of managing software without altering hashes we used it, but I think only @Atticus1806 just ran a first experiment were this could actually be relevant. But I guess in this case we could do a graph migration.

[albertz]

I noticed that we use join_right e.g. for RASR executables in our GMM pipeline, so it really has an influence on our hashes when the RASR root path has a hash_overwrite, which is actually not so uncommon.

[JackTemaki]

I noticed that we use join_right e.g. for RASR executables in our GMM pipeline, so it really has an influence on our hashes when the RASR root path has a hash_overwrite, which is actually not so uncommon.

This is exactly what I referred to when I meant only @Atticus1806 is using it. This extremely new code, so in that sense this is very uncommon.

[critias]

Resolved with #97

[Atticus1806]

As @JackTemaki already said, this breaks my newest setup. We can do a graph migration, but just wanted to give the info for documentations sake.

[critias]

Thanks for the feedback. I don't like breaking exiting setups, but I think in this case we have to since the previous behavior was wrong...