Skip to content

Add travis CI file for unit testing and lint#8

Merged
fehiepsi merged 2 commits intomasterfrom
travis-ci
Feb 27, 2019
Merged

Add travis CI file for unit testing and lint#8
fehiepsi merged 2 commits intomasterfrom
travis-ci

Conversation

@neerajprad
Copy link
Member

@neerajprad neerajprad commented Feb 27, 2019

This won't enable travis until the repo is made public, but it should resolve pytest errors. Also fixed an error with one of our distribution tests.

@neerajprad neerajprad requested a review from fehiepsi February 27, 2019 20:10
@neerajprad neerajprad changed the title Enable travis CI for unit testing and lint Add travis CI file for unit testing and lint Feb 27, 2019
@pytest.mark.parametrize("loc_scale", [
(),
(1,),
(1, 1),
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a mistake; reverting.

loc = kwargs.get('loc', args.pop() if len(args) > 0 else 0)
args = deque(args)
loc = kwargs.get('loc', args.popleft() if len(args) > 0 else 0)
scale = kwargs.get('scale', args.popleft() if len(args) > 0 else 1)
Copy link
Member

Choose a reason for hiding this comment

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

hmm, I thought that you wanted to popright here to take care of shape parameters, which appear at the beginning of some rv_continous distributions?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we should have any shape params for the logpdf method, but I think we probably do need that for the rvs method though size should still appear after loc and scale if I remember correctly. Let me check and fix that.

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue that this resolves is that norm.logpdf(x, 1.) should assign 1. to loc and not to scale (which should take its default value of 1).

Copy link
Member

Choose a reason for hiding this comment

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

Got it! I mean to use scale = kwargs.get('scale', args.pop() if len(args) > 1 else 1) here, but it seems still not be able resolve the problem when shape parameters are involved (for non-frozen distributions such as beta). So let's merge this first and think about how to deal with shape parameters later.

[tool:pytest]
filterwarnings = error
ignore:numpy.ufunc size changed,:RuntimeWarning
once:No GPU found:UserWarning
Copy link
Member

Choose a reason for hiding this comment

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

+1 Nice, I'm able to test now. Thanks @neerajprad !

@fehiepsi fehiepsi merged commit 4c46d5a into master Feb 27, 2019
@neerajprad neerajprad deleted the travis-ci branch November 19, 2019 19:03
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