-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[ENH] File: explicit file format choice #5736
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5736 +/- ##
=======================================
Coverage 86.16% 86.16%
=======================================
Files 315 315
Lines 66195 66258 +63
=======================================
+ Hits 57035 57092 +57
- Misses 9160 9166 +6 |
@borondics, I do not want to do any application of the currently selected reader onto a new file into this PR. Added automatics like this can lead to confusion (remember disappearing settings is Select Columns or Feature Constructor) so I would not add it unless absolutely sure. Furthermore, your proposed change would change the current behavior, while this PR aims to keep it exactly as it was, it only introduces the possibility of explicit modifications. The message is very precise (= not useful for non-developers), just click/mouseover on it. :) |
OK, I understand that this PR is aimed only for the new placement of the file loader selector. I hope though that in another PR the saving will be also possible. For the error: yeah, I know that there is a longer message on moouse-over but something for the normal user wouldn't hurt either. I guess there are more users than developers, so it is not absurd to also let them clearly know what is happening. :) |
bdf5087
to
48835e8
Compare
Pylint has a fun complaint: there are too many public methods (=tests) in the test class. :) |
I checked and this PR breaks nothing in Text. 👍 A small comment. It would be nice(r) if add-on readers would be listed separately. I.e. So readers from each add-on would be in a separate "category". |
4db93e4
to
aea0835
Compare
I tried to address comments by @ajdapretnar and @VesnaT: error reporting is now fixed and the list is sorted per package name. |
aea0835
to
0c712f8
Compare
9f35359
to
851b6a4
Compare
851b6a4
to
9586906
Compare
Issue
Resolves #5724
Description of changes
For files, the reader used was already stored in
.file_format
, which could be either a reader's qualified name orNone
(which means that reader is detected depending on extension and priority).All available readers are shown in a combo box.
For URLs, the reader selection is disabled. That part would have to be refactored a bit so that the chosen reader could be saved.
Includes