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

Netx #30

Merged
merged 23 commits into from Mar 1, 2022
Merged

Netx #30

merged 23 commits into from Mar 1, 2022

Conversation

bamsumit
Copy link
Contributor

@bamsumit bamsumit commented Jan 10, 2022

Issue Number: 29

Objective of pull request:
NetX implementation

Pull request checklist

Your PR fulfills the following requirements:

  • Issue created that explains the change and why it's needed
  • Tests are part of the PR (for bug fixes / features)
  • Docs reviewed and added / updated if needed (for bug fixes / features)
  • PR conforms to Coding Conventions
  • PR applys BSD 3-clause or LGPL2.1+ Licenses to all code files
  • Lint (pyb) passes locally
  • Build tests (pyb -E unit) or (python -m unittest) passes locally

Pull request type

Please check your PR type:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation changes
  • Other (please describe):

What is the current behavior?

  • Not Implemented

What is the new behavior?

  • Automatic generation of Deep networks through NetX api.

Does this introduce a breaking change?

  • Yes
  • No

Supplemental information

@bamsumit bamsumit self-assigned this Jan 10, 2022
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
@mgkwill mgkwill self-requested a review February 16, 2022 20:46
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
build.py Show resolved Hide resolved
@bamsumit bamsumit linked an issue Feb 25, 2022 that may be closed by this pull request
7 tasks
@bamsumit bamsumit added the 1-feature New feature or request label Feb 25, 2022
Copy link
Contributor

@mgkwill mgkwill left a comment

Choose a reason for hiding this comment

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

This is good work @bamsumit!

Some suggestions on docstrings, comments, TODOs in code.

Generally I don't think we should have comments in released code that is not germane to explaining something. Comments should use standard English grammar.

I.E. I think we should remove all commented code. Also TODOs should be in Github issues and not in the code itself (need to work on Lava for this also).

Signed-off-by: bamsumit <bam_sumit@hotmail.com>
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
@bamsumit
Copy link
Contributor Author

This is good work @bamsumit!

Some suggestions on docstrings, comments, TODOs in code.

Generally I don't think we should have comments in released code that is not germane to explaining something. Comments should use standard English grammar.

I.E. I think we should remove all commented code. Also TODOs should be in Github issues and not in the code itself (need to work on Lava for this also).

Thanks for the comments @mgkwill I have attended to them :)

Signed-off-by: bamsumit <bam_sumit@hotmail.com>
Signed-off-by: bamsumit <bam_sumit@hotmail.com>
@mgkwill
Copy link
Contributor

mgkwill commented Feb 28, 2022

@tim-shea are you ready to approve this?

Copy link

@tim-shea tim-shea left a comment

Choose a reason for hiding this comment

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

Great submission Sumit!

Signed-off-by: bamsumit <bam_sumit@hotmail.com>
@bamsumit bamsumit merged commit 56099b9 into lava-nc:main Mar 1, 2022
@bamsumit bamsumit deleted the netx branch March 1, 2022 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Network Exchange Implementation
3 participants