Detailed outlier flags

[vergauwenthomas]

No description provided.

[Copilot]

Pull request overview

This pull request introduces a comprehensive refactoring of the quality control (QC) outlier tracking system, replacing a dictionary-based approach with a new QCresult class. This change enables more detailed and structured tracking of QC flags, outlier timestamps, and detailed information about why records were flagged.

Changes:

• Introduces a new QCresult class to encapsulate QC check results with flags, outliers, and detailed information
• Refactors all QC check functions to return QCresult objects instead of DatetimeIndex objects
• Reorganizes the buddy check implementation into a modular structure with separate method files
• Updates SensorData class to work with QCresult objects instead of dictionaries

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 44 comments.

Show a summary per file

File	Description
src/metobs_toolkit/qcresult.py	New class defining the QCresult structure with flags, outliers, and details management
src/metobs_toolkit/sensordata.py	Updated outlier handling to use QCresult objects; added _update_outliers_NEW method
src/metobs_toolkit/qc_collection/grossvalue_check.py	Refactored to return QCresult instead of DatetimeIndex
src/metobs_toolkit/qc_collection/persistence_check.py	Refactored to return QCresult instead of DatetimeIndex
src/metobs_toolkit/qc_collection/repetitions_check.py	Refactored to return QCresult instead of DatetimeIndex
src/metobs_toolkit/qc_collection/step_check.py	Refactored to return QCresult instead of DatetimeIndex
src/metobs_toolkit/qc_collection/window_variation_check.py	Refactored to return QCresult instead of DatetimeIndex
src/metobs_toolkit/qc_collection/duplicated_timestamp.py	New function for duplicated timestamp detection returning QCresult
src/metobs_toolkit/qc_collection/invalid_check.py	New function for invalid value filtering
src/metobs_toolkit/qc_collection/common_functions.py	Added create_qcresult_flags helper function
src/metobs_toolkit/qc_collection/spatial_checks/buddywrapstation.py	New wrapper class for stations in buddy check with detailed tracking
src/metobs_toolkit/qc_collection/spatial_checks/buddy_check.py	Refactored buddy check to use new structure and return QCresult dictionary
src/metobs_toolkit/qc_collection/spatial_checks/methods/*.py	Modular organization of buddy check helper methods
src/metobs_toolkit/dataset.py	Updated buddy_check methods to work with new QCresult-based system

[Copilot]

Typo in the comment: "treath" should be "treat".

Suggested change

	# and treath them as outliers
	# and treat them as outliers

[Copilot]

The @log_entry decorator should not be used on the drop_invalid_values function as it is defined in invalid_check.py. Based on the coding guidelines, the @log_entry decorator should be applied to public methods in the main classes (like Dataset or SensorData), not to internal QC helper functions.

Suggested change

@log_entry

[Copilot]

The expression locals().pop('records', None) is problematic. The locals() function returns a dictionary that represents the current local symbol table, but modifying it (like with .pop()) does not affect the actual local variables and can lead to undefined behavior. This approach will not successfully exclude 'records' from the checksettings dictionary.

Instead, you should create a copy of locals() first, then modify that copy:

checksettings = locals().copy()
checksettings.pop('records', None)

Or build the dictionary explicitly with the values you want to include.

Suggested change

	qcresult = QCresult(
	checkname="repetitions",
	checksettings=locals().pop('records', None),
	# Build checksettings from local variables, excluding the data series itself
	checksettings = locals().copy()
	checksettings.pop("records", None)
	qcresult = QCresult(
	checkname="repetitions",
	checksettings=checksettings,

[Copilot]

The expression locals().pop('records', None) is problematic. The locals() function returns a dictionary that represents the current local symbol table, but modifying it (like with .pop()) does not affect the actual local variables and can lead to undefined behavior. This approach will not successfully exclude 'records' from the checksettings dictionary.

Instead, you should create a copy of locals() first, then modify that copy:

checksettings = locals().copy()
checksettings.pop('records', None)

Or build the dictionary explicitly with the values you want to include.

Suggested change

	qcresult = QCresult(
	checkname="gross_value",
	checksettings=locals().pop('records', None),
	checksettings = locals().copy()
	checksettings.pop('records', None)
	qcresult = QCresult(
	checkname="gross_value",
	checksettings=checksettings,

[Copilot]

The expression locals().pop('records', None) is problematic. The locals() function returns a dictionary that represents the current local symbol table, but modifying it (like with .pop()) does not affect the actual local variables and can lead to undefined behavior. This approach will not successfully exclude 'records' from the checksettings dictionary.

Instead, you should create a copy of locals() first, then modify that copy:

checksettings = locals().copy()
checksettings.pop('records', None)

Or build the dictionary explicitly with the values you want to include.

Suggested change

	qcresult = QCresult(
	checkname="persistence",
	checksettings=locals().pop('records', None),
	checksettings = locals().copy()
	checksettings.pop("records", None)
	qcresult = QCresult(
	checkname="persistence",
	checksettings=checksettings,

[Copilot]

Import of 'buddy_test_a_station' is not used.

[Copilot]

Modification of the locals() dictionary will have no effect on the local variables.