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

Force plotAssemblyTypes to always use instantiated assemblies, and al… #2030

Open
wants to merge 9 commits into
base: fix_plots_block_heights
Choose a base branch
from

Conversation

keckler
Copy link
Member

@keckler keckler commented Dec 10, 2024

This is meant to improve #1998 by allowing the specification of hot versus cold heights in the plots. Additionally, a couple clean ups are implemented:

  1. Change plotAssemblyTypes() so that it always operates based on instantiated assemblies. Allowing users to pass in blueprints was making things really messy, and provided absolutely no benefit because blueprints already contain Assembly objects in the first place through blueprints.assemblies.values().
  2. The point above means that the plotAssemblyTypes() method signature is changed. That signature is/was pretty ridiculous anyways, so I feel justified in changing it. I updated all calls to that method that I could find.

I would not expect that there are many downstream calls to this plotAssemblyTypes() method. Checking all the projects that I have access to, I found it used in exactly one place (one of my own internal repos).

@keckler
Copy link
Member Author

keckler commented Dec 10, 2024

I am relatively sure that this is working now. I tested it with a simply dummy case, however I am unable to test it with my big model due to some dependency conflicts. I think it is ready for your eyes @john-science

@keckler keckler marked this pull request as ready for review December 10, 2024 17:04
@john-science john-science added the bug Something is wrong: Highest Priority label Dec 18, 2024
@john-science john-science self-requested a review December 18, 2024 16:46
@john-science
Copy link
Member

Love it!

  1. Can we put something under the 0.5.1 release notes for "bug fixes"?
  2. Does this close a ticket we can link to it?
  3. Should we close my previous PR on the matter?

Copy link
Member

@john-science john-science left a comment

Choose a reason for hiding this comment

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

Love it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong: Highest Priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants