-
Notifications
You must be signed in to change notification settings - Fork 32
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
Addding support for other encodings than utf-8 in DownloadingOriginalFileProvider #325
base: master
Are you sure you want to change the base?
Conversation
…FileProvider.get_original_file_data by passing an encoding argument
I've converted this to draft status until discussion in #198 over in omero-scripts is finished and possibly necessary changes to the |
Launched tests. |
…original_file_data() to provide the user with more helpful error messages as well as enabling easy catching of the exception in calling scripts Removed the custom error class again, since decoders that can fail decoding will always raise a UnicodeDecodeError according to https://docs.python.org/3/library/codecs.html. Modified try-catch to add a more specific note to the UnicodeError for better user information.
I've added a try-catch in the |
…upported on all common python versions
When using this code from a script (reading a csv file) I got this as my entire sterr:
I guess the try/finally closed the file handle, then The previous code didn't close the file, so not sure you need to do that (except maybe if you actually catch an exception)? |
@will-moore |
@JulianHn I removed this from our daily build (to allow testing/merge of ome/omero-scripts#195, which was holding up your other PR at ome/omero-scripts#198)! |
@will-moore Thanks for the info will let you know. That I did not catch this myself has made me suspicious of my testing, so I want to double check things again. |
This pull request adds support for encodings differing from utf-8 in omero.utils.populate_roi.DownloadingOriginalFileProvider.get_original_file_data
This is relevant e.g. when using the Populate_Metadata.py script distributed with OMERO, when using a CSV File exported from Excel with default settings (raised as issue #323)
An accompanying pull request to omero-scripts that adds support for this new functionality will be made as well (#198).
I could not find a test for the get_original_file_data or anything from this module in /test, did I miss something?
// Julian