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

Moved leap from contrib to root folder #66

Merged
merged 4 commits into from Mar 5, 2020
Merged

Moved leap from contrib to root folder #66

merged 4 commits into from Mar 5, 2020

Conversation

adamian
Copy link
Contributor

@adamian adamian commented Feb 21, 2020

Issue #, if available:

Description of changes:
Moved xfer/xfer-ml/contrib/leap into xfer/leap. The reason for that is that the restructure has all files in flat structure under xfer, instead of inside xfer-ml (which makes sense since e.g. leap does not have dependencies in xfer-ml).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@jnkm jnkm left a comment

Choose a reason for hiding this comment

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

Can you confirm that mxnet_test.py runs and that running python setup.py install runs?

leap/README.md Outdated Show resolved Hide resolved
leap/README.md Outdated Show resolved Hide resolved
- __Install from source:__
To install leap from source, after cloning the repository run the following from the top-level directory:
```
pip install .
Copy link
Contributor

Choose a reason for hiding this comment

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

-e flag for editable for development. Potentially add separate instructions for development



## Getting Started
Check the `demos` and `test` folders.
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding link to installing jupyter/jupyter lab.

The mxnet_test seems more like a demo than a test since no checks are done and it cannot be run with any normal test runner (e.g. pytest, unittest). Maybe it can move to the demo folder?

To run the demo notebook, the user currently needs to first install leap but you can get around it with either changing the directory or adding the leap directory to the pythonpath. There doesn't really seem to be a general convention but xfer-ml requires the the package to be installed which I think is fine to continue as a convention for the repo.


[bumpversion:file:leap/__version__.py]

[coverage:run]
Copy link
Contributor

Choose a reason for hiding this comment

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

The coverage settings can be removed since there won't be CI tests running on this

README.md Show resolved Hide resolved
```
And confirm that version returned matches the expected package version number.


Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to add a request that if this code is used to please cite the paper?

@@ -1,4 +1,4 @@
# Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
# Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should all these files be changed to be 2020?

leap/README.md Outdated Show resolved Hide resolved
adamian and others added 3 commits February 25, 2020 12:23
Co-Authored-By: Jordan <jmas@live.co.uk>
Co-Authored-By: Jordan <jmas@live.co.uk>
Co-Authored-By: Jordan <jmas@live.co.uk>
@adamian adamian removed the request for review from palindromik March 5, 2020 10:48
@adamian
Copy link
Contributor Author

adamian commented Mar 5, 2020

Can you confirm that mxnet_test.py runs and that running python setup.py install runs?

They do!

@adamian adamian merged commit 1af6bc6 into master Mar 5, 2020
@adamian adamian deleted the moving_leap branch March 5, 2020 10:52
@adamian adamian mentioned this pull request Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants