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.

Sign up for GitHub

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

Jump to bottom

Feature/new ascii format #3

Merged
merged 40 commits into from
Jul 10, 2020
Merged

Feature/new ascii format #3

merged 40 commits into from
Jul 10, 2020

Conversation

eoduniyi
Copy link
Contributor

@eoduniyi eoduniyi commented Jul 9, 2020

read.txt.Witec.TrueMatch function works to import Witec ASCII/txt files exported by Witec TrueMatch. One of the key features of this function is the ability to keep extra data from the header information via the keys_2header argument:

hy_obj <- read_txt_Witec_TrueMatch(file, "none")
> hy_obj
hyperSpec object
   2 spectra
   2 data columns
   1024 data points / spectrum
wavelength: lambda/nm [numeric] 528.2085 528.3444 ... 664.3265 
data:  (2 rows x 2 columns)
   1. spc:  [matrix, array1024] 683 681 ... 726 
   2. filename: filename [character] /Users/erickoduniyi/Documents/Projects/open-source/hyperspec/hySpc.read.Witec/tests/testthat/Witec_TrueMatch.txt /Users/erickoduniyi/Documents/Projects/open-source/hyperspec/hySpc.read.Witec/tests/testthat/Witec_TrueMatch.txt

hy_obj <- read_txt_Witec_TrueMatch(file, keys_2header = c("Title", "RamanSignal", "FluorescenceSignal"))
> hy_obj
hyperSpec object
   2 spectra
   5 data columns
   1024 data points / spectrum
wavelength: lambda/nm [numeric] 528.2085 528.3444 ... 664.3265 
data:  (2 rows x 5 columns)
   1. spc:  [matrix, array1024] 683 681 ... 726 
   2. Title:  [character] Particle 916 Particle 916 
   3. RamanSignal:  [numeric] 0.4444895 0.4444895 
   4. FluorescenceSignal:  [numeric] 191.4181 191.4181 
   5. filename: filename [character] /Users/erickoduniyi/Documents/Projects/open-source/hyperspec/hySpc.read.Witec/tests/testthat/Witec_TrueMatch.txt /Users/erickoduniyi/Documents/Projects/open-source/hyperspec/hySpc.read.Witec/tests/testthat/Witec_TrueMatch.txt

cbeleites and others added 30 commits June 28, 2020 21:10
@cbeleites
Add stub file for new ASCII format
c805014
@cbeleites
Merge branch 'develop' into feature/new-ASCII-format
c398902
@cbeleites
Merge branch 'develop' into feature/new-ASCII-format
49c7d49
@cbeleites
Remove duplicate example file
f124285
@cbeleites
Merge branch 'develop' into feature/new-ASCII-format
af12adf
@cbeleites
Inital import function for new ASCII file format
c3ff935
@cbeleites
delete unittest.R from other branch
e8896c1
@cbeleites
Fix typo in documentation block
041bb08
@eoduniyi
Check performance of .txt reading functions
ae21194
@eoduniyi
Rename read_txt_Witec_ASCII to read_txt_ini_Witect
2a1194c
@eoduniyi
Switched to tidyverse naming convention
80c7ff8
@eoduniyi
Imported .txt import function from hypSpec and changed tidyverse nami…
117acc8 
…ng convention
@eoduniyi
Imported .mat import function from hypSpec and changed tidyverse nami…
4f3aae1 
…ng convention
@eoduniyi
Update NAMESPACE
2b50970
@cbeleites
Move read.ini() from hyperSpec -> hySpc.read.Witec
f34ce1a
@cbeleites
Rename read_txt_ini -> read_txt_Witec_TrueMatch to match format name
e7d157d
@cbeleites
Refactor read_txt_Witec.R into smaller files
a6a077d 
- separate file for each format
- separate file for helper functions
@cbeleites
Add example files
83b6289 
Need to go into archives since Witec Project produces file names that trigger "not portable" warnings on R CMD check.
@cbeleites
Remove old example files
73674f6 
... since we'll get new smaller ones
@cbeleites
Temporarily make .fileio.optional part of hySpc.read.Witec
a8236ab 
... until hyperSpec with exported function is publicly available
@cbeleites
Fix treatment of filename in read_txt_Witec_TrueMatch()
6b96d3e
@cbeleites
Adapt unit test to new examples
451f356 
and use Y-file as basis for filename
@cbeleites
Rename read.txt.Witec.Graph to use snake case
8942579
@cbeleites
Remove duplicate code
330630f
@cbeleites
Rename read.ini() -> read_ini()
e8eb9de
@eoduniyi
Parsing headers of Witec ASCII txt
594abc5
@eoduniyi
Change functions to snake_case
26b9c8b
@eoduniyi
Merge branch 'feature/new-ASCII-format' of https://github.com/r-hyper…
1429361 
…spec/hySpc.read.Witec into feature/new-ASCII-format
@eoduniyi
Support reading TrueMatch SpectraumData, SampleMetaData, SpectrumHeader
73151f0
@eoduniyi
Prepare test and TODOs for read_txt_Witec_TrueMatch.R
a80ae55
@eoduniyi eoduniyi requested review from cbeleites and ximeg July 9, 2020 15:34
@eoduniyi eoduniyi added the IO label Jul 9, 2020
@eoduniyi
Copy link
Contributor Author

eoduniyi commented Jul 9, 2020

Hmm not sure why it's failing on R: oldrel...RStudio says I don't have any warnings:
...test all pass fine too
Screen Shot 2020-07-09 at 10 36 23 AM

@bryanhanson
Copy link
Collaborator

The fail on old release might be something on their end. Can you re-submit (maybe just that one if you can control) and see what happens?

@eoduniyi eoduniyi requested review from bryanhanson and removed request for cbeleites and ximeg July 10, 2020 18:50
bryanhanson
bryanhanson approved these changes Jul 10, 2020
View reviewed changes
Copy link
Collaborator

@bryanhanson bryanhanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, which is to say I can build, check and install locally just fine. Approving.

bryanhanson
bryanhanson approved these changes Jul 10, 2020
View reviewed changes
Copy link
Collaborator

@bryanhanson bryanhanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I thought I just approved this but it didn't stick. Trying again.

Builds, checks and installs locally fine.

eoduniyi
eoduniyi commented Jul 10, 2020
View reviewed changes
Copy link
Contributor Author

@eoduniyi eoduniyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏿

eoduniyi
eoduniyi commented Jul 10, 2020
View reviewed changes
Copy link
Contributor Author

@eoduniyi eoduniyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏿

eoduniyi
eoduniyi commented Jul 10, 2020
View reviewed changes
Copy link
Contributor Author

@eoduniyi eoduniyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

bryanhanson
bryanhanson approved these changes Jul 10, 2020
View reviewed changes
Copy link
Collaborator

@bryanhanson bryanhanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3rd attempt to convince GH that I approve...

bryanhanson
bryanhanson approved these changes Jul 10, 2020
View reviewed changes
Copy link
Collaborator

@bryanhanson bryanhanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Viewed each file in an attempt to satisfy the monster.

bryanhanson
bryanhanson approved these changes Jul 10, 2020
View reviewed changes
Copy link
Collaborator

@bryanhanson bryanhanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Viewed each file, now trying to approve the entire lot.

@bryanhanson bryanhanson merged commit 0a6b3cf into develop Jul 10, 2020
@bryanhanson bryanhanson deleted the feature/new-ASCII-format branch July 10, 2020 21:08
@eoduniyi eoduniyi restored the feature/new-ASCII-format branch July 29, 2020 04:04
@GegznaV GegznaV deleted the feature/new-ASCII-format branch February 23, 2021 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bryanhanson bryanhanson bryanhanson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eoduniyi @bryanhanson @cbeleites