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

Display information about HModel objects (Ref issue: #22) #23

Merged
merged 1 commit into from
Jan 21, 2022

Conversation

NormannR
Copy link
Contributor

Added methods str, repr and repr_html to the HModel class

Added methods __str__, __repr__ and _repr_html_ to the HModel class
@albop
Copy link
Member

albop commented Jan 21, 2022

Thank you @NormannR . A quick question: line 119, the new version looks equivalent to the former one (d.get(k, v) is supposed to d[k] with default value v )
If it isn't that suggests a bug in the .get method. If I remember well, it is monkey patched by dolo, so that the structure returned by the yaml parsing behaves like a dict.

@NormannR
Copy link
Contributor Author

It is not exactly equivalent, as d.get() returns a ScalarNode object in our case, and ScalarNode objects do not include any get method. We then return "Anonymous" if the attribute value does not exist within this ScalarNode object. If the ScalarNode object had a get() method, we could directly use the get(k,v) syntax as you suggest.

@albop
Copy link
Member

albop commented Jan 21, 2022

Isn't model.data rather a MappingNode object? From https://github.com/EconForge/dolang.py/blob/9e9d0a47c2558656b8ae33af379d2b66965985c2/dolang/yaml_tools.py#L44, I see that it is augmented with a .get method, as soon as dolang is loaded.
It's not devoid of problem though:

  • it does not have a default value
  • you when the key is present it returns a scalarvalue object, which implies further manipulation. And might justify to keep the try/except
    So let's keep your version...

@albop albop merged commit 20f166e into EconForge:master Jan 21, 2022
@albop
Copy link
Member

albop commented Jan 21, 2022

Actually, would you mind reformatting your PR with black and commit the change ?

@NormannR
Copy link
Contributor Author

Isn't model.data rather a MappingNode object? From https://github.com/EconForge/dolang.py/blob/9e9d0a47c2558656b8ae33af379d2b66965985c2/dolang/yaml_tools.py#L44, I see that it is augmented with a .get method, as soon as dolang is loaded. It's not devoid of problem though:

* it does not have a default value

* you when the key is present it returns a scalarvalue object, which implies further manipulation. And might justify to keep the try/except
  So let's keep your version...

Indeed, model.data is a MappingNode object and model.data.name is a ScalarNodeobject, but the latter does not have a get method from which one may get the value attribute that contains what we're looking for.

Actually, would you mind reformatting your PR with black and commit the change ?

Right away!

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