Process dir #208

ZetrextJG · 2024-12-09T17:09:19Z

Implements:

process_file - process a single file E2E
process_dir - process a dir files E2E

codecov · 2024-12-09T17:12:58Z

Codecov Report

Attention: Patch coverage is 91.97531% with 13 lines in your changes missing coverage. Please review.

Project coverage is 88.58%. Comparing base (8e4bac4) to head (0343a71).
Report is 7 commits behind head on dev.

Files with missing lines	Patch %	Lines
R/process-dir.R	91.97%	11 Missing ⚠️
R/process-file.R	85.71%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #208      +/-   ##
==========================================
+ Coverage   88.19%   88.58%   +0.38%     
==========================================
  Files          14       16       +2     
  Lines        1720     1874     +154     
==========================================
+ Hits         1517     1660     +143     
- Misses        203      214      +11

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Fersoil

Some minor changes required. In general I would also add a note about process_file to the general vignette and tell the difference between different types of this process_... functions.

R/process-dir.R

R/process-file.R

Fersoil · 2024-12-11T11:40:03Z

R/process-file.R

@@ -0,0 +1,53 @@
+#' @title
+#' Process a file to generate normalised data and reports


a little more description please

R/process-file.R

R/process-dir.R

nizwant · 2024-12-21T12:31:36Z

R/process-dir.R

+#'
+#' @keywords internal
+#'
+is_mba_data_file <- function(filepath) {


I'm trying to run this on data from One drive and i get No files found in the input directory. Its because files there are named similar to IG7738~1.CSV

for this data layout is called: layout_S4_P5.xlsx

I think patterns set here are to restrective and will result in high ratio of no-match cases

The issue still persists. The format pattern that you require for the filenames is not present in our files. I guess the function is_mba_data_file should be less restrictive.

nizwant · 2024-12-21T13:14:15Z

Another interesting thing I found is that there is a case where for each plate there is different layout. For sake of convenience here is the link to the data I'm talking about: OneDrive

nizwant · 2024-12-21T14:03:42Z

I think it would be nice if list of plates returned if parametr return_plates = TRUE was sorted by date of conducting experiment - plate_datetime

Fix typos Co-authored-by: Tymoteusz Kwieciński <31191783+Fersoil@users.noreply.github.com>

ZetrextJG · 2024-12-30T12:28:13Z

I have sorted the outputs of the process_dir function with the flag return_plates set to TRUE by date from oldest to newest.

As for the layout and input file naming convention I think we have to set some kind of standard for those files if we want to process those files accordingly without any misunderstandings

Fersoil

The only problem I have noticed is too restrictive validation in is_mba_data_file, the rest looks good

Fersoil · 2025-01-02T10:16:02Z

R/process-dir.R

+#'
+#' @keywords internal
+#'
+is_mba_data_file <- function(filepath) {


The issue still persists. The format pattern that you require for the filenames is not present in our files. I guess the function is_mba_data_file should be less restrictive.

nizwant · 2025-01-02T10:54:16Z

The only problem I have noticed is too restrictive validation in is_mba_data_file, the rest looks good

There is also the issue of multiple layouts for one directory, i.e., layout per plate, but I would leave it as is and discuss it at the next meeting with Michael. He will tell us how they want to work with the package, and then we can introduce changes.

ZetrextJG · 2025-01-02T15:06:02Z

There is also the issue of multiple layouts for one directory, i.e., layout per plate

The layout per plate is supported. It will search for a layout file called {plate_csv_name}_layout.(xlsx|csv). This will activate if no layout is provided globally.

The format pattern that you require for the filenames is not present in our files

I will adopt the function so that if the format is globally provided then the pattern matching will be less restrictive

Fersoil

LGTM

* Always return the absolute path for output path * Add process file function * Move path checking to helpers * Add mock data from multiplate * Implement process dir * Normalization to normalisation * Document process_dir function * Fix typo * removed typos, added commas * fixed typos, renamed from mbr to mba * Apply suggestions from code review Fix typos Co-authored-by: Tymoteusz Kwieciński <31191783+Fersoil@users.noreply.github.com> * Create a global src config * Add more documnetation and run check * Add another example for process file * Sort returned plates by plate datetime * Always print if no plates found * Less restrictive is_mba_data_file and more output to the user * Improve test coverage --------- Co-authored-by: Tymoteusz Kwieciński <31191783+Fersoil@users.noreply.github.com> Co-authored-by: Fersoil <Fersoil>

ZetrextJG added 2 commits December 10, 2024 15:17

Always return the absolute path for output path

86b604b

Add process file function

f3e1935

ZetrextJG force-pushed the process_dir branch 2 times, most recently from fa23910 to f3e1935 Compare December 10, 2024 14:18

ZetrextJG added 6 commits December 10, 2024 23:59

Move path checking to helpers

0cdea7a

Add mock data from multiplate

05ed11c

Implement process dir

5f1b372

Normalization to normalisation

837bf01

Document process_dir function

c41d2be

Fix typo

4f4aa22

ZetrextJG marked this pull request as ready for review December 10, 2024 23:38

ZetrextJG requested a review from Fersoil December 10, 2024 23:40

removed typos, added commas

e3a9f71

Fersoil requested changes Dec 11, 2024

View reviewed changes

fixed typos, renamed from mbr to mba

8cbe5e9

nizwant reviewed Dec 21, 2024

View reviewed changes

R/process-dir.R Outdated Show resolved Hide resolved

nizwant reviewed Dec 21, 2024

View reviewed changes

This was referenced Dec 21, 2024

Stacked standard curves #212

Closed

Levey jennings plot #213

Draft

ZetrextJG and others added 5 commits December 26, 2024 21:41

Apply suggestions from code review

8987c69

Fix typos Co-authored-by: Tymoteusz Kwieciński <31191783+Fersoil@users.noreply.github.com>

Create a global src config

6b2fbcc

Add more documnetation and run check

cb68e5b

Add another example for process file

e7df2d6

Sort returned plates by plate datetime

2b0661f

Always print if no plates found

8031b48

ZetrextJG requested a review from Fersoil December 30, 2024 23:21

ZetrextJG requested a review from nizwant December 30, 2024 23:21

Fersoil requested changes Jan 2, 2025

View reviewed changes

nizwant approved these changes Jan 2, 2025

View reviewed changes

ZetrextJG added 2 commits January 2, 2025 16:43

Less restrictive is_mba_data_file and more output to the user

d84c4ff

Improve test coverage

0343a71

Fersoil self-requested a review January 3, 2025 09:33

Fersoil approved these changes Jan 3, 2025

View reviewed changes

Fersoil merged commit 35d1b27 into dev Jan 3, 2025
9 checks passed

Fersoil deleted the process_dir branch January 3, 2025 09:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Process dir #208

Process dir #208

ZetrextJG commented Dec 9, 2024

codecov bot commented Dec 9, 2024 •

edited

Loading

Fersoil left a comment

Fersoil Dec 11, 2024

nizwant Dec 21, 2024

Fersoil Jan 2, 2025

nizwant commented Dec 21, 2024

nizwant commented Dec 21, 2024

ZetrextJG commented Dec 30, 2024

Fersoil left a comment

Fersoil Jan 2, 2025

nizwant commented Jan 2, 2025

ZetrextJG commented Jan 2, 2025 •

edited

Loading

Fersoil left a comment

		@@ -0,0 +1,53 @@
		#' @title
		#' Process a file to generate normalised data and reports

Process dir #208

Process dir #208

Conversation

ZetrextJG commented Dec 9, 2024

codecov bot commented Dec 9, 2024 • edited Loading

Codecov Report

Fersoil left a comment

Choose a reason for hiding this comment

Fersoil Dec 11, 2024

Choose a reason for hiding this comment

nizwant Dec 21, 2024

Choose a reason for hiding this comment

Fersoil Jan 2, 2025

Choose a reason for hiding this comment

nizwant commented Dec 21, 2024

nizwant commented Dec 21, 2024

ZetrextJG commented Dec 30, 2024

Fersoil left a comment

Choose a reason for hiding this comment

Fersoil Jan 2, 2025

Choose a reason for hiding this comment

nizwant commented Jan 2, 2025

ZetrextJG commented Jan 2, 2025 • edited Loading

Fersoil left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 9, 2024 •

edited

Loading

ZetrextJG commented Jan 2, 2025 •

edited

Loading