FileReader #5
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: hasslesstech/IoT-Systems#5
Reference in New Issue
Block a user
Delete Branch "lab1_huranets"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
One check may cause troubles in the future and should be removed
This value range check might break future integration with higher precision accelerometer data, hence should be removed
The code passes essential CI/CD and looks ready to be merged
Misleading naming and unused variables inside of FileDatasource class
@@ -1,5 +1,9 @@from csv import readerimport csvNaming convention is misleading, suggests parser specifically takes 16-bit integers. This parser should read any integer value and should just be
int(row[0]), not a separate function callSeveral identical issues occur in the next few lines
Unused variables, seem to be leftovers from previously removed INT16 boundary checking
@@ -1,5 +1,9 @@from csv import readerimport csvremoved unnecessary variables
@@ -1,5 +1,9 @@from csv import readerimport csvMethod is not used and does not provide additional functionality compared to default
int()function, so should be removedUnneccesary check removal but is technically correct, should be fine to merge as is
Removal of checks is not neccesary, but is plausible since the source file should only contain correct data
@@ -1,5 +1,9 @@from csv import readerimport csvFixed: removed int16-specific naming/helpers and now parse accelerometer values directly using int(row[i]) without any bit-width assumptions. Pushed the update to the PR.
@@ -1,5 +1,9 @@from csv import readerimport csvRemoved the unused _parse_int helper (it didn’t add any functionality over built-in int()). Pushed the update to the PR.
Resumed checks
All immediately visible issues have been resolved
Many thanks to Oleg 'hasslesstech', for his guidance and tips during the work.
@@ -31,0 +127,4 @@continuereturn rowThis looks a bit comlex
Why itarate though loop to get 1 row?
@@ -31,0 +108,4 @@def _get_next_row(self, reader, source: str) -> List[str]:"""Get the next valid row from the reader."""if reader is None:you can just skip first row after seek(0) knowing that this is a header, not actual value,
so you can write like
self._acc_f.seek(0)
self._acc_reader = csv.reader(self._acc_f)
next(self._acc_reader)
row = next(self.acc_reader)
@@ -31,0 +108,4 @@def _get_next_row(self, reader, source: str) -> List[str]:"""Get the next valid row from the reader."""if reader is None:And do the same for other rewinders
@@ -1,5 +1,9 @@from csv import readerimport csvAnd after chage of rewinders, you can remove _acc_buf to hold first row
Some changes to simplify code
@@ -31,0 +72,4 @@self._gps_f = open(self.gps_filename, "r", newline="", encoding="utf-8")self._acc_reader = csv.reader(self._acc_f, skipinitialspace=True)self._gps_reader = csv.reader(self._gps_f, skipinitialspace=True)You just can not return this variable from _detect_header_and_buffer() if it is not used
Change function
@@ -1,5 +1,9 @@from csv import readerimport csvWhat does this means?
if assuming this is a header with indexies of domain objects maybe make sense.
question about buffer and lowering
@@ -31,0 +127,4 @@continuereturn row"Simplified the line reading logic by replacing the while True loop with a clearer check
@@ -31,0 +108,4 @@def _get_next_row(self, reader, source: str) -> List[str]:"""Get the next valid row from the reader."""if reader is None:Refactoring file rewind logic to skip header line after seek(0) and for other rewinders
@@ -1,5 +1,9 @@from csv import readerimport csvRemoved unnecessary buffers like _acc_buf and _gps_buf.
@@ -31,0 +72,4 @@self._gps_f = open(self.gps_filename, "r", newline="", encoding="utf-8")self._acc_reader = csv.reader(self._acc_f, skipinitialspace=True)self._gps_reader = csv.reader(self._gps_f, skipinitialspace=True)removed unnecessary variables from _detect_header_and_buffer() and fixed the _rewind_acc() and _rewind_gps() methods
@@ -1,5 +1,9 @@from csv import readerimport csvI initially tried removing this part, but the testing failed without it
@@ -31,0 +111,4 @@if reader is None:raise RuntimeError("Reader is not initialized.")while True:Why leave _ here if it's not used?
Maybe dont return anything from this method?
@@ -31,0 +111,4 @@if reader is None:raise RuntimeError("Reader is not initialized.")while True:As I see this method doesn't modify any class fiels, so maybe just remove this method?
@@ -31,0 +75,4 @@self._gps_reader = csv.reader(self._gps_f, skipinitialspace=True)# File pointer is already at 0 right after open(), so no need to rewind here.# Skip header row once.if you open file, your read pointer is already 0, why to set it to 0 again?
@@ -31,0 +101,4 @@def _rewind_gps(self) -> None:if self._gps_f is None:raise RuntimeError("GPS file is not open.")Why leave _ here if it's not used?
Some questions about refactoring
@@ -31,0 +111,4 @@if reader is None:raise RuntimeError("Reader is not initialized.")while True:The _detect_header_and_buffer method was not modifying any class state and its return value was unused.
I removed the method and simplified the logic.
@@ -31,0 +75,4 @@self._gps_reader = csv.reader(self._gps_f, skipinitialspace=True)# File pointer is already at 0 right after open(), so no need to rewind here.# Skip header row once.I removed the unnecessary rewind from _open_files().
@@ -31,0 +101,4 @@def _rewind_gps(self) -> None:if self._gps_f is None:raise RuntimeError("GPS file is not open.")Good catch. The _detect_header_and_buffer helper wasn’t used and didn’t modify class state, so I removed it.