-
-
Notifications
You must be signed in to change notification settings - Fork 592
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
Fixes #567 - Implement ods import #568
Conversation
Codecov Report
@@ Coverage Diff @@
## master #568 +/- ##
==========================================
+ Coverage 92.11% 92.40% +0.29%
==========================================
Files 28 28
Lines 2827 2938 +111
==========================================
+ Hits 2604 2715 +111
Misses 223 223
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
saved_registry = registry._formats.copy() | ||
try: | ||
del registry._formats['ods'] | ||
msg = (r"The 'ods' format is not available. You may want to install the " |
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.
msg = (r"The 'ods' format is not available. You may want to install the " | |
msg = (r"The ODS format is not available. You may want to install the " |
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.
If we change here, then we must also change the generation of error message in Registry.get_format
(replacing '{key}'
by {key.upper()}
). Would you suggest that? In this PR?
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.
Nice but not essential! This or another is fine :)
11746fc
to
ce04281
Compare
@hugovk Thanks for pushing me to add more tests, I also had to substantially change the code for more corner cases. |
No description provided.