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

Performance fixes #6

Merged
merged 6 commits into from
Aug 13, 2022
Merged

Performance fixes #6

merged 6 commits into from
Aug 13, 2022

Conversation

talmo
Copy link
Contributor

@talmo talmo commented Aug 3, 2022

No description provided.

@codecov
Copy link

codecov bot commented Aug 3, 2022

Codecov Report

Merging #6 (3f5fb8f) into main (8cda07f) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main        #6   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines          338       343    +5     
=========================================
+ Hits           338       343    +5     
Impacted Files Coverage Δ
sleap_io/model/instance.py 100.00% <100.00%> (ø)
sleap_io/model/labeled_frame.py 100.00% <100.00%> (ø)
sleap_io/model/labels.py 100.00% <100.00%> (ø)
sleap_io/model/skeleton.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@talmo talmo marked this pull request as ready for review August 11, 2022 16:16
tests/fixtures/generic.py Outdated Show resolved Hide resolved
Copy link
Contributor

@roomrys roomrys left a comment

Choose a reason for hiding this comment

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

We define a small fixture in test_utils, maybe we want to move that out to the fixtures/data.py file?

Comment on lines 144 to 146
def __attrs_post_init__(self):
"""Maintain point mappings between node and points after initialization."""
super().__setattr__("points", self._convert_points(None, self.points))
Copy link
Contributor

Choose a reason for hiding this comment

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

Could possibly use converters, but may be problematic since we use an instance method self._convert_points to do the conversion

Attributes can have a converter function specified, which will be called with the attribute’s passed-in value to get a new value to use.

Comment on lines 8 to 13
@pytest.fixture
def hdf5_file(tmp_path):
"""Define hdf5 fixture."""
path = str(tmp_path / "test.h5")
with h5py.File(path, "w") as f:
f.create_dataset("ds1", data=np.array([0, 1, 2]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we want all of our fixtures in the fixtures directory?

@talmo talmo merged commit 03fee32 into main Aug 13, 2022
@talmo talmo deleted the talmo/construction-perf branch August 13, 2022 02:13
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.

None yet

2 participants