Skip to content

Conversation

@Dinu23
Copy link
Contributor

@Dinu23 Dinu23 commented Sep 25, 2025

No description provided.

@Dinu23 Dinu23 requested review from Dvermetten and Copilot and removed request for Copilot September 25, 2025 09:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for reading COCO (Comparing Continuous Optimizers) data format alongside the existing JSON format support. The changes enable the inspector to parse and load COCO .info files and their associated data files.

  • Adds COCO data format parsing and loading capabilities
  • Extends file discovery to include .info files in addition to JSON files
  • Implements dedicated methods for handling COCO-specific data structures and file formats

Reviewed Changes

Copilot reviewed 3 out of 5 changed files in this pull request and generated 8 comments.

File Description
src/iohinspector/manager.py Adds COCO file discovery, parsing methods, and loading logic with bug fixes for attribute names
src/iohinspector/data.py Implements COCO text format parsing, data loading methods, and header processing utilities
pyproject.toml Updates version and pins moocore dependency to specific version

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -1,3 +1,4 @@
from email import header
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

This import appears to be accidentally added and is unused. The email module's header is unrelated to the header processing being done in this file.

Suggested change
from email import header

Copilot uses AI. Check for mistakes.
for run in scen.runs:
if run.instance not in iids:
iids.append(run.instancem)
iids.append(run.instance)
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

Fixed typo from 'run.instancem' to 'run.instance' - this was a bug fix.

Copilot uses AI. Check for mistakes.
raise FileNotFoundError(f"{coco_info_file} not found")

data_set = Dataset.from_coco_info(coco_info_file)
if(data_set is not None):
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

Unnecessary parentheses around the condition. Should be if data_set is not None:

Suggested change
if(data_set is not None):
if data_set is not None:

Copilot uses AI. Check for mistakes.
with open(self.data_file) as f:
header = process_header(next(f))
nextline = next(f).strip().split()
if(len(nextline) > len(header)):
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

Unnecessary parentheses around the condition. Should be if len(nextline) > len(header):

Suggested change
if(len(nextline) > len(header)):
if len(nextline) > len(header):

Copilot uses AI. Check for mistakes.
try:
with open(coco_info_file) as f:
data = f.read()
if( len(data.strip()) == 0):
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

Unnecessary parentheses and extra space around the condition. Should be if len(data.strip()) == 0:

Suggested change
if( len(data.strip()) == 0):
if len(data.strip()) == 0:

Copilot uses AI. Check for mistakes.
Comment on lines 395 to 397
if(len(algorithms) != 1):
raise ValueError("Multiple algorithms found in COCO text, expected one.")
if(len(function_ids) != 1):
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

Unnecessary parentheses around the conditions. Should be if len(algorithms) != 1: and if len(function_ids) != 1:

Suggested change
if(len(algorithms) != 1):
raise ValueError("Multiple algorithms found in COCO text, expected one.")
if(len(function_ids) != 1):
if len(algorithms) != 1:
raise ValueError("Multiple algorithms found in COCO text, expected one.")
if len(function_ids) != 1:

Copilot uses AI. Check for mistakes.
Comment on lines 395 to 397
if(len(algorithms) != 1):
raise ValueError("Multiple algorithms found in COCO text, expected one.")
if(len(function_ids) != 1):
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

Unnecessary parentheses around the conditions. Should be if len(algorithms) != 1: and if len(function_ids) != 1:

Suggested change
if(len(algorithms) != 1):
raise ValueError("Multiple algorithms found in COCO text, expected one.")
if(len(function_ids) != 1):
if len(algorithms) != 1:
raise ValueError("Multiple algorithms found in COCO text, expected one.")
if len(function_ids) != 1:

Copilot uses AI. Check for mistakes.
for data_set in self.data_sets:
for scen in data_set.scenarios:
df = scen.load(monotonic, data_set.function.maximization, x_values)
if(data_set.source =="coco"):
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

Unnecessary parentheses around the condition and missing space around the equality operator. Should be if data_set.source == \"coco\":

Suggested change
if(data_set.source =="coco"):
if data_set.source == "coco":

Copilot uses AI. Check for mistakes.
@Dinu23 Dinu23 requested a review from Copilot September 26, 2025 08:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 3 out of 5 changed files in this pull request and generated 5 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

for run in scen.runs:
if run.instance not in iids:
iids.append(run.instancem)
iids.append(run.instance)
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

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

Fixed typo: 'instancem' was corrected to 'instance'.

Copilot uses AI. Check for mistakes.
Comment on lines +203 to +204
if scen.dimension not in dims:
dims.append(scen.dimension)
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

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

Property name 'dim' was changed to 'dimension' but this may cause issues if the Scenario class still uses 'dim' as the property name. Verify that the Scenario class has a 'dimension' property.

Suggested change
if scen.dimension not in dims:
dims.append(scen.dimension)
dim_value = getattr(scen, "dimension", getattr(scen, "dim", None))
if dim_value is not None and dim_value not in dims:
dims.append(dim_value)

Copilot uses AI. Check for mistakes.
raise FileNotFoundError(f"{coco_info_file} not found")

data_set = Dataset.from_coco_info(coco_info_file)
if(data_set is not None):
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

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

[nitpick] Unnecessary parentheses around the condition. Python style prefers 'if data_set is not None:' without parentheses.

Suggested change
if(data_set is not None):
if data_set is not None:

Copilot uses AI. Check for mistakes.
scenarios=scenarios
)

def process_header(line:str):
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

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

[nitpick] Missing space after colon in type annotation. Should be 'line: str' according to PEP 8.

Suggested change
def process_header(line:str):
def process_header(line: str):

Copilot uses AI. Check for mistakes.

@dataclass
class Dataset:
source: str
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

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

Adding a new required field 'source' to the Dataset class is a breaking change. Consider making this field optional with a default value to maintain backward compatibility.

Suggested change
source: str
source: str = ""

Copilot uses AI. Check for mistakes.
@Dinu23 Dinu23 requested a review from Copilot September 26, 2025 12:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 67 out of 461 changed files in this pull request and generated no new comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Dinu23 Dinu23 requested a review from Copilot September 26, 2025 13:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 67 out of 461 changed files in this pull request and generated no new comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Dinu23 Dinu23 requested a review from Copilot September 26, 2025 13:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

self.assertListEqual(selection.columns, ["function_id", "data_id", "run_id", "evaluations", "raw_y"])

def test_process_header_handles_raw_y_and_evaluations(self):

Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

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

[nitpick] Extra whitespace after the colon on line 126. Remove the trailing space for consistent formatting.

Suggested change

Copilot uses AI. Check for mistakes.
Comment on lines +435 to +436
def process_header(line: str):

Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

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

Add a docstring to explain the purpose of this function, its parameters, and return value. The function processes COCO header lines and transforms them into standardized column names.

Suggested change
def process_header(line: str):
def process_header(line: str):
"""
Processes a COCO header line and transforms it into standardized column names.
Parameters
----------
line : str
The header line from a COCO data file.
Returns
-------
list of str
A list of standardized column names extracted and transformed from the header line.
"""

Copilot uses AI. Check for mistakes.
@Dinu23 Dinu23 merged commit e0585a0 into main Sep 26, 2025
3 checks passed
@jacobdenobel jacobdenobel deleted the coco-format branch December 2, 2025 14:06
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.

3 participants