Skip to content

Add data access object to sequences#364

Merged
psomhorst merged 13 commits intodevelopfrom
data_access
Apr 28, 2025
Merged

Add data access object to sequences#364
psomhorst merged 13 commits intodevelopfrom
data_access

Conversation

@psomhorst
Copy link
Contributor

@psomhorst psomhorst commented Mar 25, 2025

This PR adds a data interface to sequences for easier access to data objects. Instead of sequence.continuous_data['filtered impedance'] one can now use sequence.data['filtered impedance']. Adding data is also easier with `sequence.data.add(object)

Other dict function are also implemented, such that sequence.data can be used as a conglomeration of all data collections.

This improves readability of end user code. The old interface is still intact and usable. To use the interface, all labels have to be unique across all data collections.

Fixes #363

@psomhorst psomhorst self-assigned this Mar 25, 2025
@psomhorst
Copy link
Contributor Author

To be discussed: an even shorter way is to add sequence.d['filtered impedance']. This reduces the amount of characters needed, but introduces a level of abstraction. I don't think I like it enough to propose it. What do you think, @JulietteFrancovich?

Copy link
Contributor

@JulietteFrancovich JulietteFrancovich left a comment

Choose a reason for hiding this comment

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

I think this is a very useful addition. I have added some minor comments.

To be discussed: an even shorter way is to add sequence.d['filtered impedance']. This reduces the amount of characters needed, but introduces a level of abstraction. I don't think I like it enough to propose it. What do you think, @JulietteFrancovich?

I personally think winning three characters does not weigh up against the decrease in readability when shortening .data to .d. I would refrain from doing so.

for a, b in itertools.combinations(self._collections, 2):
if duplicates := set(a) & set(b):
msg = (
f"Duplicate labels ({', '.join(sorted(duplicates))}) found in {a} and {b}. "
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding what someone can do (use original interface).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion!

I've added this as an explanation in the two exceptions about duplicate labels.
(This will only show up in Python 3.11 or higher, because I'm using the notes feature of exceptions introduced in Python 3.11]. I think it's best to keep the exception message short and clear; suggestions for better approaches are also given as exception notes in popular libraries.)

with pytest.raises(KeyError):
sequence.data.add(interval_data_object)

# you can still add through the DataCollection, but this interface will not be available anymore
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this something that is also explained to the user in for instance a docstring?

@psomhorst psomhorst merged commit 662e617 into develop Apr 28, 2025
3 checks passed
@psomhorst psomhorst deleted the data_access branch April 28, 2025 19:53
@psomhorst psomhorst mentioned this pull request Jul 9, 2025
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.

2 participants