Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature to directly open files with IrodsPath #241

Merged
merged 4 commits into from
Aug 19, 2024
Merged

Conversation

qubixes
Copy link
Collaborator

@qubixes qubixes commented Aug 14, 2024

This will make it more natural for users to open files directly from the IrodsPath, instead of needing to go through the PRC.

Fixes #170

TODO:

  • Documentation
  • Tests
  • Tutorials

@qubixes qubixes requested a review from chStaiger August 15, 2024 08:45
@qubixes qubixes marked this pull request as ready for review August 15, 2024 08:45
Copy link
Collaborator

@chStaiger chStaiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice new feature!
There is only one thing we need to further investigate: writing of existing data objects.

if self.collection_exists():
raise ValueError("Cannot open collection, only data objects can be opened.")
# Create the data object if it does not exist.
if mode == "w" and not self.dataobject_exists():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add that existing object cannot be opened in write mode. It would be very useful as it led to inconsistencies when I tried that previously!!!

Can we maybe add an else which throws a message that an existing object can only be opened in read mode?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe we need to test a bit more for the writing existing data objects.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems inconsistent with how opening files works though. Doesn't it make more sense that the data object gets removed and then opened for writing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chStaiger I have tested, and writing to a data object that already exists is possible and will simply overwrite it. I think this is how it should be done. Let me know if you agree, then you can perhaps approve and merge.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think I opened the data object in "append" mode. And that led to problems. So all fine with me.

@chStaiger chStaiger self-requested a review August 18, 2024 17:39
Copy link
Collaborator

@chStaiger chStaiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woohoo.

@chStaiger chStaiger merged commit f365a8b into develop Aug 19, 2024
9 checks passed
@chStaiger chStaiger deleted the open-ipath branch August 19, 2024 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make IrodsPath usable with open() function
2 participants