Skip to content

add script to convert states #136

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

jeffery9
Copy link

No description provided.

@jeffery9 jeffery9 force-pushed the 17-states branch 8 times, most recently from 68eb7b6 to 886f3fb Compare April 23, 2025 15:44
Copy link
Contributor

@hailangvn hailangvn left a comment

Choose a reason for hiding this comment

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

Thank you for the work. Please check my comments. Thanks.

"""
Convert `states` to XML attributes (readonly, required, invisible).
"""
logger.info(f"Parsing states: {states}") # 添加日志,打印解析到的 states
Copy link
Contributor

Choose a reason for hiding this comment

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

Please translate the comment to English.

field_name = target.id
states_value = None
for keyword in getattr(node.value, "keywords", []):
if keyword.arg == "states":
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is better to skip below logic if it is not states.

Suggested change
if keyword.arg == "states":
if keyword.arg != "states":
continue

Comment on lines +440 to +449
if isinstance(keyword.value, ast.Dict):
# Direct dictionary
states_value = ast.literal_eval(keyword.value)
elif isinstance(keyword.value, ast.Name):
# Variable reference
var_name = keyword.value.id
if var_name in variable_definitions:
states_value = ast.literal_eval(
variable_definitions[var_name]
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if isinstance(keyword.value, ast.Dict):
# Direct dictionary
states_value = ast.literal_eval(keyword.value)
elif isinstance(keyword.value, ast.Name):
# Variable reference
var_name = keyword.value.id
if var_name in variable_definitions:
states_value = ast.literal_eval(
variable_definitions[var_name]
)
if isinstance(keyword.value, ast.Dict):
# Direct dictionary
states_value = ast.literal_eval(keyword.value)
elif isinstance(keyword.value, ast.Name):
# Variable reference
var_name = keyword.value.id
if var_name in variable_definitions:
states_value = ast.literal_eval(
variable_definitions[var_name]
)

if not xml_attrs:
logger.info(
f"No valid attributes generated from states: {states}"
) # 添加日志,打印未生成有效属性的情况
Copy link
Contributor

Choose a reason for hiding this comment

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

Please translate the comment to English.


# Write the updated XML back to the file
with open(xml_file, "wb") as f:
tree.write(f, pretty_print=True, encoding="utf-8", xml_declaration=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

This line causes test failure because lxml.etree is hard coded to write header with single quote. It is better to only write when there is change.

Suggested change
tree.write(f, pretty_print=True, encoding="utf-8", xml_declaration=True)
tree.write(f, pretty_print=True, encoding="utf-8",
doctype='<?xml version="1.0" encoding="utf-8"?>')

Comment on lines +423 to +424
if current_model:
model_field_mapping[current_model] = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

These two lines are not needed as model_field_mapping[current_model] is not used anywhere and current_model is part of the key used in model_field_mapping.

Comment on lines +548 to +551
# Create a copy of arch_root to modify independently
arch_root_copy = et.Element(arch_root.tag, arch_root.attrib)
for child in arch_root:
arch_root_copy.append(child)
Copy link
Contributor

@hailangvn hailangvn May 16, 2025

Choose a reason for hiding this comment

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

I could not see the benefit of arch_root_copy. Below code prove that it moves, not copies, children from arch_root to arch_root_copy.

Suggested change
# Create a copy of arch_root to modify independently
arch_root_copy = et.Element(arch_root.tag, arch_root.attrib)
for child in arch_root:
arch_root_copy.append(child)
# Create a copy of arch_root to modify independently
arch_root_copy = et.Element(arch_root.tag, arch_root.attrib)
for child in arch_root:
arch_root_copy.append(child)
logger.info(
f"arch_root_copy, {arch_root_copy}, {len(arch_root_copy)}"
)
logger.info(f"arch_root, {arch_root}, {len(arch_root)}")

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.

None yet

2 participants