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

Move classes and public functions to top of files #1227

Closed
5 tasks
Tracked by #1198
guarin opened this issue May 10, 2023 · 7 comments
Closed
5 tasks
Tracked by #1198

Move classes and public functions to top of files #1227

guarin opened this issue May 10, 2023 · 7 comments

Comments

@guarin
Copy link
Contributor

guarin commented May 10, 2023

Description

  • Move classes and exported functions to top of file, move private functions to bottom
  • Do this on a per subpackage basis

Private functions or classes start with an underscore:

  • def _private_fun
  • def _PrivateClass
  • def public_fun
  • def PublicClass

Exceptions are types/type aliases that should remain next to the function/class where they are used.

Tasks

  • Move public classes and functions to top in lightly/data
  • Move public classes and functions to top in lightly/loss
  • Move public classes and functions to top in lightly/models/modules
  • Move public classes and functions to top in lightly/transforms
  • Move public classes and functions to top in lightly/utils
@guarin guarin mentioned this issue May 10, 2023
18 tasks
@guarin guarin added the package label Aug 16, 2024
@moli-debugger
Copy link

@guarin I have moved the private func to bottom level for visibility and accessibility. kindly create a seprate issue for moving the private func to the bottom to the mentioned files and assign it to me.

  1. lightly/models/byol.py
  2. lightly/models/nnclr.py
  3. lightly/models/utils.py

@guarin
Copy link
Contributor Author

guarin commented Oct 7, 2024

@moli-debugger thanks for looking into this! I created the issue here: #1671 I can assign it to you once you comment on it.

FYI the SSL models in lightly/models like model/simclr.py etc. are deprecated and will be removed in the next major version update. I'll update the issue description with a list of files that are most important to update.

@fadkeabhi
Copy link
Contributor

I would also like to work on this issue, If there are any more files that need to be changed I would like to do it.

@guarin
Copy link
Contributor Author

guarin commented Oct 8, 2024

Hi @fadkeabhi, thanks for looking into this! I added some more directories to the issue description. Would you be interested on any of them?

@fadkeabhi
Copy link
Contributor

@guarin Thank you for the opportunity. I will work on those directories. Will submit a PR regarding it soon.

@fadkeabhi
Copy link
Contributor

@guarin I have submitted a PR for All the above mentioned directories. Take a look at it here #1685.
lightly/transforms did not need any changes as it was already cleaned.

@SauravMaheshkar
Copy link
Collaborator

Closing as completed in #1685

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

4 participants