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

Make thumbs up robust for empty / small catalogs #186

Merged
merged 2 commits into from
Dec 7, 2018
Merged

Conversation

taldcroft
Copy link
Member

Closes #185.

Also bumps version to the next (maybe last?) feature release.

@taldcroft taldcroft added the bug Something isn't working label Dec 7, 2018
@taldcroft taldcroft requested a review from jeanconn December 7, 2018 20:22
@@ -182,6 +182,8 @@ def fid_set(self, fid_ids):
def thumbs_up(self):
if self.n_acq == 0:
out = 1
elif len(self) < 2:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine. I wasn't sure if we wanted to add the p_acq column to the empty AcqTable (with similar changes for empty fid or guide tables) or handle with logic like this for the three kinds of tables.

@jeanconn
Copy link
Contributor

jeanconn commented Dec 7, 2018

Ok. With this change I can cut this on the matlab side

try
    pcat.Acq.thumbs_up = pyexec('tu = int(catalog.acqs.thumbs_up)', 'tu');
    pcat.Guide.thumbs_up = pyexec('tu = int(catalog.guides.thumbs_up)', 'tu');
    pcat.Fid.thumbs_up = pyexec('tu = int(catalog.fids.thumbs_up)', 'tu');
    pcat.thumbs_up = pyexec('tu = int(catalog.thumbs_up)', 'tu');
catch exception
    warning(exception.message)
    pcat.thumbs_up = 0;
    pcat.Fid.thumbs_up = 0;
    pcat.Guide.thumbs_up = 0;
    pcat.Acq.thumbs_up = 0;
end

@jeanconn jeanconn merged commit df9ae84 into master Dec 7, 2018
@jeanconn jeanconn deleted the empty-thumbs branch December 7, 2018 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants