-
Notifications
You must be signed in to change notification settings - Fork 21.7k
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
[DOCS] Fixed KLDiv example #126857
[DOCS] Fixed KLDiv example #126857
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/126857
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 5528fd9 with merge base 8a45979 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
torch/nn/modules/loss.py
Outdated
@@ -449,6 +449,8 @@ class KLDivLoss(_Loss): | |||
Examples:: | |||
|
|||
>>> import torch.nn.functional as F | |||
>>> from torch import nn | |||
>>> import torch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should maybe import torch
be moved above import torch.nn.functional as F
? also, style of from torch import nn
does not match the style of import torch.nn.functional as F
I would propose (or at least make them the same style):
import torch
import torch.nn as nn
import torch.nn.functional as F
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey!
Actually you should even remove the functional import from here as these are expected to always be already done in all our docstrings as you can see in the doctest config here:
Lines 912 to 914 in 082251e
"from torch import nn", | |
"import torch.nn.functional as F", | |
"import torch", |
remove extra \n
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: Not merging any PRs at the moment because there is a merge blocking https://github.com/pytorch/pytorch/labels/ci:%20sev issue open at: Details for Dev Infra teamRaised by workflow job |
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Small import fix to make the example run Pull Request resolved: pytorch#126857 Approved by: https://github.com/albanD
Small import fix to make the example run