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

Update config_parser.py #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marcelopessoa1
Copy link

in DHALSIM/dhalsim/parser/config_parser.py

The Setup in installation has erros because of changes in yamlinclude:

So, I've changed the file "DHALSIM/dhalsim/parser/config_parser.py " in:

  • Line 9:
  • from yamlinclude import YamlIncludeConstructor
  • from yaml_include import Constructor

and

  • In line 629:
  • Constructor.add_to_loader_class(loader_class=yaml.FullLoader, base_dir=config_path.absolute().parent)
  • yaml.add_constructor("!include", Constructor(base_dir=config_path.absolute().parent), FullLoader)

It worked for me!

in DHALSIM/dhalsim/parser/config_parser.py 

The Setup in installation has erros because of changes in yamlinclude:

* It seems that yamlinclude changed to yaml_include: https://pyyaml-include.readthedocs.io/en/main/README.html:
* In  2.0.a1 release (Date 2024-1-27) the tag constructor class YamlIncludeConstructor renamed to Constructor (https://pyyaml-include.readthedocs.io/en/main/CHANGELOG.html)
* the classmethod YamlIncludeConstructor.add_to_loader_class() was removed from [pyyaml-include 2.0's Constructor](https://github.com/tanbro/pyyaml-include/blob/v2.0.1/src/yaml_include/constructor.py). 

So, I've changed the file "DHALSIM/dhalsim/parser/config_parser.py " in:

* Line 9:

- `from yamlinclude import YamlIncludeConstructor`
+ `from yaml_include import Constructor`

and
* In line 629:
- `Constructor.add_to_loader_class(loader_class=yaml.FullLoader,
                                                   base_dir=config_path.absolute().parent)`
+  `yaml.add_constructor("!include", Constructor(base_dir=config_path.absolute().parent), FullLoader)`

It worked for me!
@afmurillo
Copy link
Collaborator

Hello @marcelopessoa1! First of all, thank you very much for your interest and helping with DHALSIM. Sorry that I have not had time to take a look at this, I think it looks good, but I need to test it before approving, I will try to do so in July.

@afmurillo
Copy link
Collaborator

afmurillo commented Jul 10, 2024

Hello @marcelopessoa1,

I checked your changes, but they did not work for me. Could you modify slightly your PR for the version that worked for me?

in config_parser.py I did:

Line 9:

# from yamlinclude import YamlIncludeConstructor
import yaml_include

Then line 630 became:

# YamlIncludeConstructor.add_to_loader_class(loader_class=yaml.FullLoader,
#                                            base_dir=config_path.absolute().parent)
yaml.add_constructor("!include", yaml_include.Constructor(base_dir=config_path.absolute().parent), yaml.FullLoader)

Also, keep in mind that I also did a merge from /dev into /master and that version reverted back pyyaml-include to 1.41:
in setup.py 'pyyaml-include==1.4.1', so make sure to install pyaml-include==2.1 while testing your patch. Sorry for these recent changes, but I would like to include your PR into the repo :)

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