Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add prefilter option to context reference property #1468

Conversation

jberends
Copy link
Member

@jberends jberends commented Sep 11, 2024

Fixes #

Checklist:

  • My code follows the pykechain style
    • I added type hinting to all functions
    • I added documentation for all public functions
    • I locally used tox to check for dists and docs errors
    • My code passes the pep8 and flake8 linting checks and no warnings
    • I removed unused imports, using Code > Optimize Imports in PyCharm
    • I used black to format the new code
  • I have added tests that prove my fix is effective or that my feature works
    • I committed fresh test cassettes
    • I have proper test coverage (don't decline the coverage of the test)
  • I asked another teammate to review the code
  • I update the Changelog.md with the appropriate changes.

Jochem Berends added 6 commits September 11, 2024 14:36
Implemented a method to set prefilters on the context reference property and created a new ContextgroupFilter class. Updated relevant imports and code sections to support this functionality.
Correct the indentation of property type checks to enhance code readability and maintain consistency. This change ensures proper alignment and structure in the conditional statements.
…erties-in-value-options

# Conflicts:
#	CHANGELOG.rst
Implemented `set_prefilters` method in `ContextReferencesProperty` to support clearing and setting the `context_group` prefilter. Added corresponding unit tests to ensure proper functionality and to handle edge cases, including invalid inputs.
Refactor the exception raise to eliminate the unnecessary f-string formatting since no variables are being interpolated. This cleanup simplifies the code and ensures better readability.
@jberends jberends requested a review from KaczuH September 16, 2024 10:13
@jberends jberends self-assigned this Sep 16, 2024
@jberends jberends linked an issue Sep 16, 2024 that may be closed by this pull request
Jochem Berends added 4 commits September 16, 2024 12:15
Deleted the ContextGroupFilter class including its methods and references to the ContextGroup enum. Corrected typos in comments for clarity.
Adjusted indentation to ensure that property types in the filter are correctly and clearly aligned. This enhances readability and maintains consistency in the code formatting structure.
Simplified the docstring of `parse_options` method for better readability. Changed the description to be more concise and removed unnecessary line breaks.
Removed the import of `ContextType` and `ContextReferencesProperty` from `test_properties.py` as they were unused. This change reduces unnecessary code clutter and improves maintainability of the test suite.
Copy link
Contributor

@KaczuH KaczuH left a comment

Choose a reason for hiding this comment

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

I have few concerns.

pykechain/models/property_reference.py Show resolved Hide resolved
pykechain/models/property_reference.py Show resolved Hide resolved
pykechain/models/property_reference.py Show resolved Hide resolved
@jberends jberends merged commit 9026f67 into main Nov 26, 2024
6 of 7 checks passed
@jberends jberends deleted the add-contextgroup-prefilter-on-reference-properties-in-value-options branch November 26, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

add prefilters option to context reference property
2 participants