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

shortening filter fix #198

Merged
merged 1 commit into from
Sep 11, 2020
Merged

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Aug 5, 2020

Fixes #194

Another python 2 > 3 fix. Use integer division to fix error from issue above.

To test:

Screenshot 2020-08-05 at 15 39 31

@will-moore will-moore changed the title Fix shortening filter, use integer division shortening filter fix Aug 5, 2020
Copy link
Member

@manics manics left a comment

Choose a reason for hiding this comment

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

LGTM.

You could optionally replace ... with a single unicode ellipsis to allow more characters (or alternatively to add a space before/after), but that's not required.

@joshmoore
Copy link
Member

I assume it would be good to roll this into a release, no? (I also wonder if there's any potential linting that would help us detect this type of error)

@will-moore
Copy link
Member Author

It's not particularly urgent - I think it can wait for the next release, whenever that is.
For the user, it's a silent failure, and you just see the full name for companion files instead of the truncated one.

@will-moore
Copy link
Member Author

I guess it would be nice to detect this kind of TypeErrors, but don't know if it's possible.
On the plus side, we don't tend to introduce these types of errors during development (as far as I'm aware!) - just still catching one's from Python 2 -> 3 migration.

@will-moore
Copy link
Member Author

#203 attempts to catch remaining integer division errors.

@sbesson sbesson merged commit 18d18db into ome:master Sep 11, 2020
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.

TypeError in common_filters
4 participants