-
Notifications
You must be signed in to change notification settings - Fork 13
Samplingfeaturedataset dev #143
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
Conversation
@lsetiawan: I'll let you review this PR; I'm too tied up today, then gone. I see that it's a PR on the dev branch, so it won't impact the new release (and won't go into the new release, to be issued today). |
I'm ready to go with the release (it's in draft form right now). @lsetiawan, do you think you'll have time to review and merge into master this PR? I have a couple more NicaDIF things for you :| |
@Elijahwalkerwest I noticed that you are not meeting a lot of Python PEP8 Rules, so I've made it more visible. I am doing this so no one has to go back and clean up again for Python Pep8 errors. So please fix your code to meet PEP8. You will see the errors here: https://travis-ci.org/ODM2/ODM2PythonAPI/jobs/331031358 |
That's it. I'm releasing from master as is, w/o this PR. Sorry @Elijahwalkerwest! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see #143 (comment)
@ocefpaf Do you know why the Docs creation is failing here? Thanks!
|
@lsetiawan I checked and the link seems to be OK now, so you can restart Travis-CI to ensure that or, if you know the context of that link in docs and a server error is not important, just merge it 😉 |
Thanks @ocefpaf. That makes sense, didn't know it's doing that. Cool! :D |
All is well now, and I have tested the changes. Merging... Thanks @Elijahwalkerwest |
Added eager loading for samplingfeaturedataset to try and cut down on query time.