Refactor BOM generation to centralize logic #177
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As it turns out, System Capture BOM generation did not get the benefits of the changes in #157 as the functions for filtering and sorting rows were not called in the system capture BOM function. This was an entirely avoidable mistake as the logic for BOM generation for all formats is substantially the same. This commit moves the logic for BOM generation into the `generate_bom` function and makes the format specific functions aliases of the same. This should avoid similar mistakes in the future.
Coverage SummaryTotal Project Coverage
Coverage by File
Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
Diff Coverage Detailsallspice/utils/list_components.pyLines 289-297 289 return SupportedTool.ORCAD
290 elif source_file.lower().endswith(".sdax"):
291 return SupportedTool.SYSTEM_CAPTURE
292 else:
! 293 raise ValueError("""
294 The source file for generate_bom must be:
295
296 - A PrjPcb file for Altium projects; or
297 - A DSN file for OrCAD projects; or |
McRaeAlex
approved these changes
Aug 28, 2024
McRaeAlex
reviewed
Aug 28, 2024
| if remove_non_bom_components: | ||
| project_tool = infer_project_tool(source_file) | ||
| if project_tool == SupportedTool.ALTIUM: | ||
| components = _remove_non_bom_components(components) |
There was a problem hiding this comment.
Unless the plan is to use the same function for the other file formats I think we should prefix this as altium specific.
McRaeAlex
approved these changes
Aug 28, 2024
kdumontnu
approved these changes
Aug 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As it turns out, System Capture BOM generation did not get the benefits of the changes in #157 as the functions for filtering and sorting rows were not called in the system capture BOM function. This was an entirely avoidable mistake as the logic for BOM generation for all formats is substantially the same. This commit moves the logic for BOM generation into the
generate_bomfunction and makes the format specific functions aliases of the same. This should avoid similar mistakes in the future.Closes ID-529.