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

Siqi_scratch #16

Merged
merged 32 commits into from
Jun 11, 2019
Merged

Siqi_scratch #16

merged 32 commits into from
Jun 11, 2019

Conversation

iqis
Copy link
Contributor

@iqis iqis commented May 6, 2019

@wendtke

I've taken a rather radical approach in modifying the code, please do bear with me. You don't need to merge everything from the pull request.

Before considering this pull request, I would suggest downloading this siqi_scratch branch, do a build on your own machine, and play around with it.

You can try out the only function (so far) meant to be user-facing through the following:

EDA_data <- psyphr::read_MW_EDA("01-1-1.xlsx")

This will return a list with three data frames: EDA Stats, SCR Stats, Editing Stats, and a named character vector Settings, corresponding to the four different sheets of the original Excel workbook.

Please pay no regard to the warnings such as:

Warning messages:
1: In fansi::strwrap_ctl(x, width = max(width, 0), indent = indent,  :
  Encountered a C0 control character, see `?unhandled_ctl`; you can use `warn=FALSE` to turn off these warnings.

This comes from parsing the contents and will be turned off in the future.

In the attribute fields of the list, I also include device_vendor ("MindWare"), origin_path, and origin_mtime to document the file path and modified time for future use. I'm sure they will be helpful when the user constructs a study.

You may also notice that I've taken a very modular approach in making the functions. I believe this is beneficial because when we expand the functionality of the package to more data types, we can have easily reusable parts. This is however, very opinionated, and I welcome a challenge from any angle.

Additionally, I shall note the following:

  • I have, for the time, deleted the actual tests from my branch. As much as I love automated testing, and constantly nag people about how great it is, I believe right now would be a little premature to take full advantage of them; the current tests will fail the build if you try to build the package.
  • Among the commit messages, ro man means generating manual pages for the functions with roxygen2.
  • I have initialized a README page for future use, stealing a little from your Wiki Page input. Please add, delete and modify as you see fit. This is important, however, as when we open the repo up to the public, this is the first thing visitors will see.

Thank you ~~~~~

@iqis iqis assigned wendtke and iqis May 6, 2019
@iqis iqis mentioned this pull request May 6, 2019
@wendtke
Copy link
Owner

wendtke commented May 7, 2019

Thank you so much, @iqis! I look forward to breaking this all down and examining your approach when I have more time. End-of-semester crunch time. :/

fix typos; add description and links
@wendtke wendtke merged commit 0e5a43f into master Jun 11, 2019
@wendtke
Copy link
Owner

wendtke commented Jun 11, 2019

Good to delete branch? And Audrey's and Amanda's?

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

Successfully merging this pull request may close these issues.

2 participants