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

Use only dots to delimit controls #9942

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

yngve-sk
Copy link
Contributor

@yngve-sk yngve-sk commented Feb 3, 2025

Issue
Resolves #9816

Approach
Keep backward compatibility (should be deprecated?) but accept only . notation to delimit control names, variable/index etc

@yngve-sk yngve-sk force-pushed the 25.02.03.make-names-consistent branch from 068c4ad to 2d25df7 Compare February 3, 2025 11:45
Copy link

codspeed-hq bot commented Feb 3, 2025

CodSpeed Performance Report

Merging #9942 will improve performances by 10.15%

Comparing yngve-sk:25.02.03.make-names-consistent (3738bc8) with main (978727e)

Summary

⚡ 1 improvements
✅ 24 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
test_load_from_context[gen_x: 20, sum_x: 20 reals: 10] 7 ms 6.4 ms +10.15%

@yngve-sk yngve-sk force-pushed the 25.02.03.make-names-consistent branch 5 times, most recently from e6241f0 to b084afc Compare February 4, 2025 11:11
@yngve-sk yngve-sk self-assigned this Feb 4, 2025
Copy link
Contributor

@verveerpj verveerpj left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines 117 to 126
def _get_control_index(name: str):
try:
matching_index = formatted_control_names.index(name.replace("-", "."))
return matching_index
except ValueError:
pass

return formatted_control_names_dotdash.index(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Eventually this should disappear, since using the dash is deprecated. Maybe a comment?

@yngve-sk yngve-sk force-pushed the 25.02.03.make-names-consistent branch from b084afc to 6f66d73 Compare February 5, 2025 11:02
Copy link
Contributor

@verveerpj verveerpj left a comment

Choose a reason for hiding this comment

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

LGTM!

@yngve-sk yngve-sk force-pushed the 25.02.03.make-names-consistent branch from 6f66d73 to 3738bc8 Compare February 5, 2025 12:40
@yngve-sk yngve-sk merged commit 8482458 into equinor:main Feb 5, 2025
26 checks passed
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.

Make control naming consistent
2 participants