Skip to content

Conversation

@psomhorst
Copy link
Contributor

@psomhorst psomhorst commented Jun 3, 2024

While working on tests for ContinuousData, I was fixing some locking mechanisms. Since these will probably be reused in the future, I decided to write a mixin instead.

This mixin has been implemented and tested with ContinuousData only.

@psomhorst psomhorst changed the base branch from main to 167_sparse_data_psomhorst June 3, 2024 12:56
@psomhorst
Copy link
Contributor Author

On naming: the Lockable mixin class and the lock(), unlock(), islocked() and islockable() methods might suggest that the entire object is made lockable. This is not the case. Attributes in the object are lockable. We could rename this to:

  • lock_attr()
  • unlock_attr()
  • attr_is_locked()
  • attr_is_lockable()

but that becomes quite verbose.

@psomhorst psomhorst closed this Jun 7, 2024
@psomhorst psomhorst deleted the lockable_mixin branch June 7, 2024 08:20
@DaniBodor
Copy link
Member

DaniBodor commented Jun 10, 2024

Could you comment on why this PR was closed (without merging)?

EDIT: I now see (or at least assume) that it has been superceded by #215.
Generally it's nice to have a short comment on why a PR/Issue was closed without being accepted. Even if it's just a reference to a new PR/issue.

@psomhorst
Copy link
Contributor Author

@DaniBodor This was auto-closed by GitHub when I changed the branch name. I thought it would transfer the PR to the new branch name, but that turned out to not be the case. I was unable to reopen the commit and change the branch afterwards.

@DaniBodor
Copy link
Member

Ah, i understand. I'm also surprised, it usually does transfer it in my experience

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.

3 participants