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

Fix yaml serialization in io mixin #11106

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Fix yaml serialization in io mixin #11106

merged 2 commits into from
Oct 31, 2024

Conversation

hemildesai
Copy link
Collaborator

Uses class only when data.qualname or similar fails

Copy link
Collaborator

@cuichenx cuichenx left a comment

Choose a reason for hiding this comment

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

Would be good if you could add some comments or doc strings to this function too, thanks!

call = False
except Exception:
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it possible to specify the exception types here? or is it difficult to enumerate

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can change it to AttributeError

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed in bd903a0

Signed-off-by: Hemil Desai <[email protected]>
@hemildesai
Copy link
Collaborator Author

Add docstring

Updated in bd903a0

@oyilmaz-nvidia oyilmaz-nvidia enabled auto-merge (squash) October 31, 2024 00:06
Copy link
Contributor

[🤖]: Hi @hemildesai 👋,

We wanted to let you know that a CICD pipeline for this PR just finished successfully

So it might be time to merge this PR or get some approvals

I'm just a bot so I'll leave it you what to do next.

//cc @pablo-garay @ko3n1g

@pablo-garay
Copy link
Collaborator

@pablo-garay pablo-garay disabled auto-merge October 31, 2024 02:28
@pablo-garay
Copy link
Collaborator

There was a further commit after the triggered CI run but it was a docstring name + name change which shouldnt affect result:
bd903a0

@pablo-garay pablo-garay merged commit ab7b325 into main Oct 31, 2024
15 checks passed
@pablo-garay pablo-garay deleted the hemil/fix-yaml branch October 31, 2024 02:46
ko3n1g pushed a commit that referenced this pull request Oct 31, 2024
* Fix yaml serialization in io mixin

Signed-off-by: Hemil Desai <[email protected]>

* Add docstring

Signed-off-by: Hemil Desai <[email protected]>

---------

Signed-off-by: Hemil Desai <[email protected]>
pablo-garay pushed a commit that referenced this pull request Oct 31, 2024
* Fix yaml serialization in io mixin



* Add docstring



---------

Signed-off-by: Hemil Desai <[email protected]>
Co-authored-by: Hemil Desai <[email protected]>
hainan-xv pushed a commit to hainan-xv/NeMo that referenced this pull request Nov 5, 2024
* Fix yaml serialization in io mixin

Signed-off-by: Hemil Desai <[email protected]>

* Add docstring

Signed-off-by: Hemil Desai <[email protected]>

---------

Signed-off-by: Hemil Desai <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>
lilyw97 pushed a commit to lilyw97/NeMo that referenced this pull request Nov 13, 2024
* Fix yaml serialization in io mixin

Signed-off-by: Hemil Desai <[email protected]>

* Add docstring

Signed-off-by: Hemil Desai <[email protected]>

---------

Signed-off-by: Hemil Desai <[email protected]>
HuiyingLi pushed a commit to HuiyingLi/NeMo that referenced this pull request Nov 15, 2024
* Fix yaml serialization in io mixin

Signed-off-by: Hemil Desai <[email protected]>

* Add docstring

Signed-off-by: Hemil Desai <[email protected]>

---------

Signed-off-by: Hemil Desai <[email protected]>
XuesongYang pushed a commit to paarthneekhara/NeMo that referenced this pull request Jan 18, 2025
* Fix yaml serialization in io mixin

Signed-off-by: Hemil Desai <[email protected]>

* Add docstring

Signed-off-by: Hemil Desai <[email protected]>

---------

Signed-off-by: Hemil Desai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants