Skip to content

Remove attribute is_merged #140

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

Merged
merged 3 commits into from
Jan 7, 2023
Merged

Conversation

fjosw
Copy link
Owner

@fjosw fjosw commented Jan 6, 2023

This PR removes the Obs attribute is_merged which is no longer needed after the removal of _filter_zeroes. On my machine, on python 3.8 this results in a ~1.15x speed up for the multiplication of two Obs.

@fjosw fjosw requested a review from s-kuberski January 6, 2023 12:45
@s-kuberski
Copy link
Collaborator

Given the speed up, we should probably get rid of this attribute... Therefore, this is fine with me, although I liked to have this extra piece of information in the past.

@fjosw
Copy link
Owner Author

fjosw commented Jan 7, 2023

I agree, that the speed-up outweighs the missing info. I hope that this change does not break any workflows. The only case in which I see problems is trying to unpickle an old pyerrors 2.x pickle file with the new version but I think we made clear that this should be avoided anyways.

@fjosw fjosw merged commit a77cf22 into develop Jan 7, 2023
@fjosw fjosw deleted the feat/remove_is_merged_attribute branch January 7, 2023 10:23
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