-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add reader for ascii files #34
Conversation
cc4829a
to
c3e6419
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #34 +/- ##
===========================================
- Coverage 91.04% 90.08% -0.97%
===========================================
Files 7 8 +1
Lines 1106 1170 +64
===========================================
+ Hits 1007 1054 +47
- Misses 99 116 +17 ☔ View full report in Codecov by Sentry. |
rdata/parser/_xdr.py
Outdated
*, | ||
expand_altrep: bool = True, | ||
altrep_constructor_dict: AltRepConstructorMap = DEFAULT_ALTREP_MAP, | ||
**kwargs: Any, |
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.
I would rather keep the parameters everywhere (for type-checking). Alternatively, you could type it with an unpacked TypedDict as in PEP 692, but I think it does not worth it in this case.
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.
I added the parameters back (d193450), but without default values so that they are not duplicated from _parser.py
here.
d3e3aa5
to
af24a29
Compare
Line ending of |
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.
LGTM
@vnmabus Thanks for reviewing and merging! |
This PR adds parser for ascii files.
See also #31.
A minimal test is also added for reading the following files generated in R:
These files are generated both on Linux and Windows. Windows-R seems to use \r\n line ending for rds files but \n for rda files (tested on R 4.3.2).
@vnmabus Could you review? Do you think that this kind of testing would be sufficient?