Skip to content

Conversation

@samwaseda
Copy link
Member

@samwaseda samwaseda commented Sep 8, 2025

Based on today's discussion, I implemented a function to translate input arguments to the names, while the positional only arguments are translated to numbers

@github-actions
Copy link

github-actions bot commented Sep 8, 2025

Binder 👈 Launch a binder notebook on branch pyiron/flowrep/keyword

@samwaseda samwaseda requested a review from Copilot September 8, 2025 20:14
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 a new function _get_function_keywords to parse function signatures and translate parameter information, where positional-only arguments are represented as numeric indices and other arguments are represented by their names.

  • Added _get_function_keywords function that inspects function signatures and returns parameter representations
  • Added corresponding unit test to verify the function behavior with different parameter types

Reviewed Changes

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

File Description
flowrep/workflow.py Implements the new _get_function_keywords function using inspect module
tests/unit/test_workflow.py Adds unit test for the new function with positional-only, regular, and keyword-only parameters

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

}


def _get_function_keywords(function: Callable) -> dict[str, Any]:
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

The function name _get_function_keywords is misleading since it returns a list, not just keywords. The return type annotation should be list[Union[int, str]] instead of dict[str, Any] to accurately reflect that it returns a list containing either integers (for positional-only params) or strings (for named params).

Copilot uses AI. Check for mistakes.
items.append(ii)
else:
items.append(name)
return items
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

The function returns a list but the type annotation indicates dict[str, Any]. This mismatch will cause type checking issues. Change the return type to list[Union[int, str]] to match the actual return value.

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Sep 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.17%. Comparing base (e36ea57) to head (6af16f7).
⚠️ Report is 6 commits behind head on minimal.

Additional details and impacted files
@@             Coverage Diff             @@
##           minimal      #31      +/-   ##
===========================================
- Coverage    96.29%   96.17%   -0.12%     
===========================================
  Files            2        2              
  Lines          593      602       +9     
===========================================
+ Hits           571      579       +8     
- Misses          22       23       +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@samwaseda samwaseda merged commit 8cde18a into minimal Sep 8, 2025
16 of 18 checks passed
@samwaseda samwaseda deleted the keyword branch September 8, 2025 20:41
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.

2 participants