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

Fixes issue #719, savReaderWriter including headers in data #720

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

geirfreysson
Copy link
Contributor

I don't know if you guys are having this problem but whenever I try and read an SPSS file with the latest savReaderWriter it throws an error because it includes the first line (the header) in the data being read.

This pull request is a fix for that. We check for the version of savReaderWriter being used and if it is a recent one, we start reading after line 0 (i.e. the header).

We check for the version of savReaderWriter being used and if it is a recent one, we start reading after line 0 (i.e. the header).
@jamesrkg jamesrkg self-assigned this Feb 27, 2017
@jamesrkg jamesrkg added this to the RG-80 milestone Feb 27, 2017
@jamesrkg
Copy link
Contributor

@geirfreysson we're not on the most recent version but I'm going to take a look at this. We've done some lib updates recently and it would be better for us to update if we can.

@jamesrkg
Copy link
Contributor

jamesrkg commented Mar 6, 2017

@geirfreysson can we achieve this without adding packaging as a dependency?

>>> import savReaderWriter as srw
>>> srw.version
'3.3.0'

@jamesrkg jamesrkg modified the milestones: RG-81, RG-80 Mar 6, 2017
@geirfreysson
Copy link
Contributor Author

geirfreysson commented Mar 7, 2017

Yes, it can be done without the 3rd party package. Do you think, @jamesrkg, that I should replace the packaging method with distutils.version?

According to this stack overflow answer:

Use distutils.version or packaging.version.parse.

Differences between the two options:

  • distutils.version is built in but is undocumented and conformant only to the superseded PEP 386;
  • packaging.version.parse is a third-party utility but is used by setuptools (so you probably already have it installed) and is conformant to the current PEP 440; it also handles "loose" and "strict" versions in a single function (although "legacy" versions will always sort before valid versions).

@jamesrkg jamesrkg modified the milestones: RG-81, RG-84 Mar 27, 2017
@jamesrkg
Copy link
Contributor

@geirfreysson I'll add packages as a dependency and merge this PR on Friday. 👍

@geirfreysson
Copy link
Contributor Author

geirfreysson commented Mar 27, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants