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

bugfix CNV write, fixes #2001 #2010

Merged
merged 3 commits into from Oct 9, 2018

Conversation

Projects
None yet
3 participants
@ThomasLecocq
Contributor

ThomasLecocq commented Nov 18, 2017

Passing phase_mapping to Catalog.write(format="CNV", phase_mapping=...) resulted in an exception.

Bugfix #2001

This #2009 is not linked but could be part of the same PR?

@ThomasLecocq ThomasLecocq requested a review from megies Nov 18, 2017

@ThomasLecocq

This comment has been minimized.

Show comment
Hide comment
@ThomasLecocq

ThomasLecocq Nov 18, 2017

Contributor

How do I disable the warnings when S phases don't have a mapping in the last test?

Contributor

ThomasLecocq commented Nov 18, 2017

How do I disable the warnings when S phases don't have a mapping in the last test?

@krischer

This comment has been minimized.

Show comment
Hide comment
@krischer

krischer Nov 20, 2017

Member

warnings.catch_warnings() - you can also use this to test the raised warning and if it is an expected warning it is a good idea to do so.

Member

krischer commented Nov 20, 2017

warnings.catch_warnings() - you can also use this to test the raised warning and if it is an expected warning it is a good idea to do so.

@megies

megies approved these changes Nov 20, 2017

Looks good, can be merged once the warnings in tests are handled properly.

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies Nov 20, 2017

Member

Oh.. @ThomasLecocq should be rebased on maintenance_1.1.x!

Member

megies commented Nov 20, 2017

Oh.. @ThomasLecocq should be rebased on maintenance_1.1.x!

@megies megies changed the base branch from master to maintenance_1.1.x Nov 20, 2017

@ThomasLecocq

This comment has been minimized.

Show comment
Hide comment
@ThomasLecocq

ThomasLecocq Nov 22, 2017

Contributor

@megies did you rebase or just changed the "base branch" of the PR ?

Contributor

ThomasLecocq commented Nov 22, 2017

@megies did you rebase or just changed the "base branch" of the PR ?

@ThomasLecocq

This comment has been minimized.

Show comment
Hide comment
@ThomasLecocq

ThomasLecocq Nov 22, 2017

Contributor

I think I rebased on maintenance_1.1.x but then why does it show stuff from you... dunno...

Contributor

ThomasLecocq commented Nov 22, 2017

I think I rebased on maintenance_1.1.x but then why does it show stuff from you... dunno...

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies Nov 23, 2017

Member

I think I rebased on maintenance_1.1.x but then why does it show stuff from you... dunno...

I don't think this worked.. those commits are from master and they're not even the original ones but re-commited.. 😜

I can rebase your branch if you like, but you'll need to check it out fresh from here then..?

Member

megies commented Nov 23, 2017

I think I rebased on maintenance_1.1.x but then why does it show stuff from you... dunno...

I don't think this worked.. those commits are from master and they're not even the original ones but re-commited.. 😜

I can rebase your branch if you like, but you'll need to check it out fresh from here then..?

@ThomasLecocq

This comment has been minimized.

Show comment
Hide comment
@ThomasLecocq

ThomasLecocq Nov 23, 2017

Contributor

Pleeeaaase do!!!

Contributor

ThomasLecocq commented Nov 23, 2017

Pleeeaaase do!!!

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies Nov 23, 2017

Member

@ThomasLecocq please double check. :-)

Member

megies commented Nov 23, 2017

@ThomasLecocq please double check. :-)

ThomasLecocq and others added some commits Nov 18, 2017

adds warning catching and test their content + number
workaround for checking list contents
@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies Oct 8, 2018

Member

Rebased.. can we merge this @ThomasLecocq?

Member

megies commented Oct 8, 2018

Rebased.. can we merge this @ThomasLecocq?

@megies megies added this to the 1.1.1 milestone Oct 8, 2018

@ThomasLecocq

This comment has been minimized.

Show comment
Hide comment
@ThomasLecocq

ThomasLecocq Oct 9, 2018

Contributor

Yep, if test pass, I guess it's good to go.

Appveyor error seem unrelated:

IOError: [Errno 2] No such file or directory: 'epsg'
Command exited with code 1

looks lilke pyproj's dependency epsg wasn't installed...

Contributor

ThomasLecocq commented Oct 9, 2018

Yep, if test pass, I guess it's good to go.

Appveyor error seem unrelated:

IOError: [Errno 2] No such file or directory: 'epsg'
Command exited with code 1

looks lilke pyproj's dependency epsg wasn't installed...

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies Oct 9, 2018

Member

Yeah some conda packaging issues for proj on win it seems

Member

megies commented Oct 9, 2018

Yeah some conda packaging issues for proj on win it seems

@megies megies merged commit 6b76dea into maintenance_1.1.x Oct 9, 2018

2 of 4 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
docker-testbot docker testbot results not available yet
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@megies megies deleted the fix_cnv_write branch Oct 9, 2018

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