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

avoid generic except #57

Closed
wants to merge 0 commits into from

Conversation

ocefpaf
Copy link
Collaborator

@ocefpaf ocefpaf commented Mar 20, 2018

The generic expect may trigger the second calculation when we should've failed early anyway.

@ocefpaf
Copy link
Collaborator Author

ocefpaf commented Mar 20, 2018

@wesleybowman I'm planning to make a new release, v0.2.2, once this is merged. Is that OK?

@wesleybowman
Copy link
Owner

Absolutely 👍

Copy link
Collaborator

@efiring efiring left a comment

Choose a reason for hiding this comment

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

Good--except that it looks like there is an indentation inconsistency (2 spaces instead of 4).

@ocefpaf
Copy link
Collaborator Author

ocefpaf commented Mar 20, 2018

Thanks @efiring! I missed that in #56

@efiring
Copy link
Collaborator

efiring commented Mar 20, 2018

Would someone give me write access to this repo, please?

@ocefpaf
Copy link
Collaborator Author

ocefpaf commented Mar 20, 2018

Would someone give me write access to this repo, please?

I guess that only @wesleybowman can do that.

@efiring
Copy link
Collaborator

efiring commented Mar 20, 2018

Also, this PR illustrates that it would be good to have some minimal (not full PEP8) "must pass" checking: 4 spaces, no trailing whitespace.

@wesleybowman
Copy link
Owner

@efiring I had sent you an invite awhile back (back when I also added @ocefpaf)
screenshot from 2018-03-21 07-18-58

Here is the invite link:
https://github.com/wesleybowman/UTide/invitations

@ocefpaf ocefpaf closed this Mar 21, 2018
@ocefpaf ocefpaf deleted the avoid_generic_except branch March 21, 2018 14:46
@ocefpaf
Copy link
Collaborator Author

ocefpaf commented Mar 21, 2018

The commit here are in #58

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants