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

Fix injection of added parameters #1981

Merged
merged 3 commits into from
Oct 31, 2024
Merged

Fix injection of added parameters #1981

merged 3 commits into from
Oct 31, 2024

Conversation

aulemahal
Copy link
Collaborator

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes #xyz
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • CHANGELOG.rst has been updated (with summary of main changes)
    • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

The issue was that a YAML definition of an indicator was not able to inject (set) the indexer parameter on indicators that inherited from IndexingIndicator (i.e. indicators that rely on a class-level implementation of the indexing, instead of compute-level one).

This reorganizes how these class-added arguments are handled. It also renames them "added" arguments instead of "injected" to clear the confusion. "injection" now only means an indicator argument that is set when creating a derived indicator (that what most YAML definitions do). "added" is an argument that is not in the compute function, but added by the class.

The example YAML is modified and a test is added using this modification.

Does this PR introduce a breaking change?

No.

The feature was not really documented and now it works instead of not.

Other information:

@aulemahal aulemahal requested a review from Zeitsperre October 30, 2024 22:09
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added the docs Improvements to documenation label Oct 30, 2024
@github-actions github-actions bot added the approved Approved for additional tests label Oct 31, 2024
@coveralls
Copy link

coveralls commented Oct 31, 2024

Coverage Status

coverage: 89.377%. remained the same
when pulling d2be937 on fix-indexer-injection
into 5bf3e9b on main.

@Zeitsperre Zeitsperre merged commit 5e7bd3d into main Oct 31, 2024
21 checks passed
@Zeitsperre Zeitsperre deleted the fix-indexer-injection branch October 31, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests docs Improvements to documenation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants