Skip to content

Conversation

@Robbybp
Copy link
Member

@Robbybp Robbybp commented Mar 6, 2024

These should be imported from pyomo.common.numeric_types. Also, pyomo_constant_types was deprecated last week (although I don't do anything about that). This PR is necessary to prevent import errors with pyomo/main, as pyomo_constant_types no longer exists in pyomo.core.expr.numvalue.

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@codecov
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.59%. Comparing base (fc1a7da) to head (1951915).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1366      +/-   ##
==========================================
- Coverage   77.60%   77.59%   -0.01%     
==========================================
  Files         391      391              
  Lines       64330    64330              
  Branches    14244    14244              
==========================================
- Hits        49923    49920       -3     
- Misses      11831    11835       +4     
+ Partials     2576     2575       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@andrewlee94 andrewlee94 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@ksbeattie ksbeattie added the Priority:Normal Normal Priority Issue or PR label Mar 7, 2024
@ksbeattie ksbeattie requested a review from jsiirola March 7, 2024 19:31
Copy link
Contributor

@dallan-keylogic dallan-keylogic left a comment

Choose a reason for hiding this comment

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

Looks good to me. If pyomo_constant_types is deprecated, we probably should open an issue to move away from using it eventually.

@andrewlee94 andrewlee94 enabled auto-merge (squash) March 11, 2024 15:31
@jsiirola
Copy link
Contributor

jsiirola commented Mar 11, 2024

I would also recommend changing the following line (and removing the import of pyomo_constant_types):

diff --git a/idaes/core/util/scaling.py b/idaes/core/util/scaling.py
index d4bd825..f3ee1cf 100644
--- a/idaes/core/util/scaling.py
+++ b/idaes/core/util/scaling.py
@@ -1509,7 +1509,7 @@ class NominalValueExtractionVisitor(EXPR.StreamBasedExpressionVisitor):
         # first check if the node is a leaf
         nodetype = type(node)

-        if nodetype in native_types or nodetype in pyomo_constant_types:
+        if nodetype in native_types:
             return [node]

         node_func = self.node_type_method_map.get(nodetype, None)

@andrewlee94 andrewlee94 disabled auto-merge March 11, 2024 15:35
@andrewlee94
Copy link
Contributor

@Robbybp Would you be able to add the addition change @jsiirola suggested?

@lbianchi-lbl lbianchi-lbl enabled auto-merge (squash) March 13, 2024 04:01
@lbianchi-lbl lbianchi-lbl merged commit 38e67db into IDAES:main Mar 13, 2024
@Robbybp Robbybp deleted the consttype-import branch March 13, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority:Normal Normal Priority Issue or PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants