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

Allow unicode strings for subsetting. #1941

Merged
merged 3 commits into from Apr 26, 2016

Conversation

Projects
None yet
4 participants
@danlipsa
Contributor

danlipsa commented Apr 20, 2016

This is how they come from the web.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Apr 20, 2016

wow there are still some types floating around! I thought I removed them all! Maybe we should check the rest of the file(s) for use of types and clean this up as well

Allow unicode strings for subsetting.
This is how they come from the web.

@danlipsa danlipsa force-pushed the cdat-web-plot-subsetting branch from e94f7c8 to 585e438 Apr 21, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Apr 21, 2016

Sure, I can do the string ones. What do you do for the rest of the types?

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Apr 21, 2016

https://gitlab.kitware.com/UV-CDAT/uvcdat/merge_requests/614
@doutriaux1 @aashish24 I think this is ready. We can clean up in a different PR.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Apr 22, 2016

Dashboard looks good to me thanks to @sankhesh' effort. LGTM 👍

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Apr 25, 2016

@aashish24 @sankhesh is that the url for buildbot results? https://gitlab.kitware.com/UV-CDAT/uvcdat/merge_requests/614/builds ? I only see kitware there, I thought we had at least oceanonly and crunchy going.

@sankhesh

This comment has been minimized.

Contributor

sankhesh commented Apr 25, 2016

@doutriaux1 Nope. Buildbot auto-generates a custom CDash link for builds specific to the merge request and posts a comment on the merge request.

Here is an example: https://gitlab.kitware.com/UV-CDAT/uvcdat/merge_requests/614#note_90623

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Apr 25, 2016

thanks @sankhesh

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Apr 25, 2016

@danlipsa there's failure on the few bots we have.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Apr 25, 2016

@doutriaux1 Are you talking about oceanonly? Those tests fail on master as well. There is the diag tests that fail for some time and recently I started seeing some vcs_verify tests failing because they cannot find a py file.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Apr 26, 2016

@doutriaux1 Ready to merge?

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Apr 26, 2016

@danlipsa while we are at it I would like to clean this up a bit more, I'll push an updated version in 10mn

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Apr 26, 2016

@doutriaux1 We could always do the clean-up as a separate PR. I would rather do it that way instead of delaying this any further.

@danlipsa danlipsa merged commit 1c84b48 into master Apr 26, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Apr 26, 2016

@doutriaux1 This is the issue so that we don't forget
#1950

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Apr 26, 2016

@danlipsa ok just pushed the changes I wanted. Please review and merge if tests pass for you.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Apr 26, 2016

@danlipsa I see you already merged it... Oh well...

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Apr 26, 2016

@doutriaux1 Just make another PR and I can review it and merge it quickly.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Apr 26, 2016

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