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

Problem with num iterations in Dr16FixedEtaVarlssFudgeExpectedFlux #985

Closed
cramirezpe opened this issue Mar 24, 2023 · 1 comment
Closed

Comments

@cramirezpe
Copy link
Contributor

Since default values cannot be changed by a child class (line 328):

def update_default_options(default_options, new_options):
"""Update the content of the list of accepted options
Arguments
---------
default_options: dict
The current default options
new_options: dict
The new options
Return
------
default_options: dict
The updated default options
"""
default_options = default_options.copy()
for key, value in new_options.items():
if key in default_options:
default_value = default_options.get(key)
if type(default_value) is not type(value):
raise DeltaExtractionError(
f"Incompatible defaults are being added. Key {key} "
"found to have values with different type: "
f"{type(default_value)} and {type(value)}. "
"Revise your recent changes or contact picca developpers.")
if default_value != value:
raise DeltaExtractionError(
f"Incompatible defaults are being added. Key {key} "
f"found to have two default values: '{value}' and '{default_value}' "
"Revise your recent changes or contact picca developpers.")
else:
default_options[key] = value
return default_options

This means that:

defaults = update_default_options(defaults, {
"num iterations": 1,
})

Will always fail with the error:

picca.delta_extraction.errors.DeltaExtractionError: Incompatible defaults are being added. Key num iterations found to have two default values: '1' and '5' Revise your recent changes or contact picca developpers.

Because the Dr16ExpectedFlux sets "num iterations"=5.

@iprafols iprafols mentioned this issue Mar 24, 2023
@iprafols
Copy link
Collaborator

I added a quick fix to this issue in PR #986 @cramirezpe please let me know if you are happy with the fix

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

No branches or pull requests

2 participants