-
Notifications
You must be signed in to change notification settings - Fork 515
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
Reformat layers/
directory with black
#199
Conversation
I was finding it hard to read the slice notation in the `FactorizedSpectralConvNd.forward` implementations (i.e. where `out_fft[*] = self._contract(x[*], ...)`) for what's supposed to be the same slice `[*]`. Refactor each call to `self._contract` to use the same slice to index each tensor (i.e. to index `out_fft` and `x`).
Additionally, rename function arg `type` to `skip_type` to not overshadow builtin `type()`
@JeanKossaifi I've split formatting changes out from earlier commits here to reduce bloat in those PRs. I'll rebase #186 on top of this to clean it up. |
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 @m4e7 looks good to me, just one comment about reqs.
requirements.txt
Outdated
zarr | ||
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.
@m4e7 the reason I hadn't included torch in the requirements is that typically users don't want to install it with pip (conda, Docker etc typically are better to handle CUDA installs) and this can cause issues particularly with pip -U
where it may override existing install with wrong version.
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.
Aha, got it. I didn't realize that was a common user workflow. I've removed that line from requirements.txt
Reformat `layers/` directory with `black`
Reformat select files (in particular, those changes in #186) in directory
neuralop/layers
withblack
.Additionally, add a "Contributing" section to the README, and state
black
as a formatting tool.