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

Check if object is already a DICT before it is passed to JSON5.loads for parsing #401

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

Conversation

LostQuant
Copy link

@LostQuant LostQuant commented May 1, 2024

🤔 What is the nature of this change? / 这个变动的性质是?

  • New feature / 新特性提交
  • Fix bug / bug 修复
  • Refactor code or style / 重构代码或样式
  • Performance optimization / 性能优化
  • Build optimization / 构建优化
  • Website, documentation, demo improvements / 网站、文档、Demo 改进
  • Test related / 测试相关
  • Other / 其他

🔗 Related Issue / 相关 Issue

#396

💡 Background or solution / 需求背景和解决方案

In many places when using JSON5.loads to parse a serialized object, it doesnt check if the argument passed is a STR. The PR is to fix it by assuming the object is not serialized and only do json5.loads if it turns out to be a STRING.

📝 Changelog / 更新日志

Default behavior is preserved, no breaking changes/risks.

@LostQuant LostQuant changed the title Bypass json5 load subtasks in plan_exec.py unless its a string Bypass json5.loads when reading subtasks in plan_exec.py unless its a string May 1, 2024
@LostQuant LostQuant changed the title Bypass json5.loads when reading subtasks in plan_exec.py unless its a string Check if object is already a DICT before it is passed to JSON5.loads for parsing May 1, 2024

Choose a reason for hiding this comment

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

hello

Choose a reason for hiding this comment

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

revert

Copy link
Author

Choose a reason for hiding this comment

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

revert

Hi, sry if I missed anything but i dont see any comments

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.

2 participants