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

Test Case for Issue #1932 (read_events failing with Unicode chars in file path) #2361

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@DominikStr
Copy link
Contributor

DominikStr commented Mar 17, 2019

Adds an example file with umlauts to the test data and a testcase failing if a `UnicodeDecodeError' gets raised.

What does this PR do?

Contains a test case for issue #1932

Why was it initiated? Any relevant Issues?

See (read_events fails with unicode characters in file path #1932)

PR Checklist

  • Correct base branch selected? master for new features, maintenance_... for bug fixes
  • This PR is not directly related to an existing issue (which has no PR yet).
  • If the PR is making changes to documentation, docs pages can be built automatically.
    Just remove the space in the following string after the + sign: "+ DOCS"
  • If any network modules should be tested for the PR, add them as a comma separated list
    (e.g. clients.fdsn,clients.arclink) after the colon in the following magic string: "+TESTS:"
    (you can also add "ALL" to just simply run all tests across all modules)
  • All tests still pass.
  • Any new features or fixed regressions are be covered via new tests.
  • Any new or changed features have are fully documented.
  • Significant changes have been added to CHANGELOG.txt .
  • First time contributors have added your name to CONTRIBUTORS.txt .
@megies

This comment has been minimized.

Copy link
Member

megies commented Mar 19, 2019

@DominikStr can you please rebase on current master and force-push to this PR? That should get rid of the Circle CI fails.

@megies

This comment has been minimized.

Copy link
Member

megies commented Mar 19, 2019

Also no need to make a separate issue for the fix, just add the fix here as well. Thanks!

@DominikStr DominikStr force-pushed the DominikStr:fix_1932_unicode_characters_in_file_path branch from 0a302ce to 3a6dd76 Mar 20, 2019

@DominikStr

This comment has been minimized.

Copy link
Contributor Author

DominikStr commented Mar 27, 2019

I force-pushed it but I can't see what else to fix to get it through the circleci test.

@megies

This comment has been minimized.

Copy link
Member

megies commented Mar 28, 2019

@DominikStr these CircleCI fails are fixed on current master. Can you rebase please and force-push once more? Thanks!

@megies megies changed the title Test Case for Issue #1932 Test Case for Issue #1932 (read_events failing with Unicode chars in file path) Mar 28, 2019

@megies megies added the .core.event label Mar 28, 2019

DominikStr added some commits Mar 12, 2019

Test Case for Issue #1932
Adds an example file with umlauts to the test data and a testcase failing if a `UnicodeDecodeError' gets raised.

@DominikStr DominikStr force-pushed the DominikStr:fix_1932_unicode_characters_in_file_path branch from 3a6dd76 to 8616c30 Mar 28, 2019

@@ -223,6 +223,8 @@ def setUp(self):
self.image_dir = os.path.join(os.path.dirname(__file__), 'images')
self.iris_xml = os.path.join(path, 'iris_events.xml')
self.neries_xml = os.path.join(path, 'neries_events.xml')
unicode_path = "unicode_char_test_äöüß/unicode_test_catalog_äöüß.xml"

This comment has been minimized.

Copy link
@krischer

krischer Apr 11, 2019

Member

Please also use os.path.join() here to make sure it works on all systems.

@krischer

This comment has been minimized.

Copy link
Member

krischer commented Apr 11, 2019

Failures are unrelated - expect the one small issue I have this is IMHO ready to merge.

@QuLogic

This comment has been minimized.

Copy link
Member

QuLogic commented Apr 12, 2019

Are you sure the test failure is unrelated? It appears to be about "special" characters in file paths, which is what this new file is exercising.

Now, it is unrelated to the issue, but I'm not sure it's unrelated to this PR.

@krischer

This comment has been minimized.

Copy link
Member

krischer commented Apr 12, 2019

Are you sure the test failure is unrelated? It appears to be about "special" characters in file paths, which is what this new file is exercising.

Now, it is unrelated to the issue, but I'm not sure it's unrelated to this PR.

Yes I think you are right! I did not read the failures carefully enough and the new test file drips up the obspy.core.util.testing.get_all_py_files() function.

@DominikStr can you locally test this on Python 2.7 and try to fix it? On Python 3 it will just work as unicode strings are the default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.