Skip to content

Combine split utils - #1333

Open
xjjiang wants to merge 11 commits into
OpenMDAO:mainfrom
xjjiang:combine_split_utils
Open

Combine split utils#1333
xjjiang wants to merge 11 commits into
OpenMDAO:mainfrom
xjjiang:combine_split_utils

Conversation

@xjjiang

@xjjiang xjjiang commented Oct 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Currently, there are three utility files in utils folder:

  • conversion_utils.py
  • math_utils.py
  • utils.py

conversion_utils.py is merged into utils.py and math_utils.py stays.

Related Issues

Backwards incompatibilities

None

AI Usage

None

@xjjiang xjjiang changed the title [Draft] Combine split utils Combine split utils Oct 14, 2026
@xjjiang
xjjiang marked this pull request as ready for review October 14, 2026 15:56
@Kenneth-T-Moore

Kenneth-T-Moore commented Oct 16, 2026

Copy link
Copy Markdown
Member

I think all the math related utils should be separated into utils/math.py. The others are generic enough to stay.

@jkirk5

jkirk5 commented Oct 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

I think all the math related utils should be separated into utils/math.py. The others are generic enough to stay.

That name causes confusion when also using Python's math, so I explicitly gave that file a different name a while back to avoid that

@xjjiang

xjjiang commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

so I explicitly gave that file a different name a while back to avoid that

I did use the name you gave: math_utils.py.

@jkirk5
jkirk5 enabled auto-merge October 22, 2026 19:38
@jkirk5

jkirk5 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Was conversion_utils.py supposed to be deleted? I don't see it in the diff

auto-merge was automatically disabled October 22, 2026 20:15

Head branch was pushed to by a user without write access

@xjjiang

xjjiang commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Was conversion_utils.py supposed to be deleted? I don't see it in the diff

Oops! It is deleted now.

cmbenne3
cmbenne3 approved these changes Oct 26, 2026
View reviewed changes
Comment thread aviary/subsystems/mass/mass_builder.py
val, units = find_variable_in_problem(Aircraft.Nacelle.MASS, prob, self.meta_data)
f.write(f'|Nacelles|{np.dot(val, num_engines)}||\n')
if val != 'Not Found in Model':
f.write(f'|Nacelles|{np.dot(val, num_engines)}||\n')

@cmbenne3 cmbenne3 Oct 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we like conditionality in our reports? This is fine here as it is consistent with the landing gear code above... I will add an issue to the repo #1344

@cmbenne3 cmbenne3 mentioned this pull request Oct 26, 2026
Open

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmbenne3 cmbenne3 cmbenne3 approved these changes
@Kenneth-T-Moore Kenneth-T-Moore Awaiting requested review from Kenneth-T-Moore

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Combine split util files

4 participants

@xjjiang @Kenneth-T-Moore @jkirk5 @cmbenne3