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

Repo creation #101

Merged
merged 11 commits into from Aug 4, 2019
Merged

Repo creation #101

merged 11 commits into from Aug 4, 2019

Conversation

rlizzo
Copy link
Member

@rlizzo rlizzo commented Aug 4, 2019

Motivation and Context

Why is this change required? What problem does it solve?:

Fixes issues with inconsistent warnings and adds more user control and more sane type annotations

If it fixes an open issue, please link to the issue here:

Description

Describe your changes in detail:

  • exists parameter added to Repository() which if set to False will suppress the warning that a repo does not exist at the given path.
  • changed Hangar directory to .hangar instead of __hangar as per [BUG REPORT] New repo creation is unfriendly #93
  • added README.txt inside .hangar folder to aid in repo identification
  • added --version option to CLI.

Types of changes

What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Documentation update
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Is this PR ready for review, or a work in progress?

  • Ready for review
  • Work in progress

How Has This Been Tested?

Put an x in the boxes that apply:

  • Current tests cover modifications made
  • New tests have been added to the test suite
  • Modifications were made to existing tests to support these changes
  • Tests may be needed, but they are not included when the PR was proposed
  • I don't know. Help!

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have signed (or will sign when prompted) the tensorwork CLA.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov
Copy link

codecov bot commented Aug 4, 2019

Codecov Report

Merging #101 into master will increase coverage by 0.05%.
The diff coverage is 93.16%.

@@            Coverage Diff             @@
##           master     #101      +/-   ##
==========================================
+ Coverage   91.78%   91.83%   +0.05%     
==========================================
  Files          45       46       +1     
  Lines        8184     8454     +270     
  Branches      819      834      +15     
==========================================
+ Hits         7511     7763     +252     
- Misses        492      508      +16     
- Partials      181      183       +2
Impacted Files Coverage Δ
src/hangar/backends/hdf5_00.py 86.54% <ø> (-0.58%) ⬇️
src/hangar/backends/remote_50.py 100% <ø> (ø) ⬆️
src/hangar/backends/numpy_10.py 94.74% <ø> (-0.08%) ⬇️
src/hangar/records/hashs.py 72.83% <ø> (-0.86%) ⬇️
src/hangar/records/commiting.py 92.64% <0%> (-0.46%) ⬇️
src/hangar/records/parsing.py 97.67% <100%> (+0.23%) ⬆️
tests/test_dataset.py 100% <100%> (ø) ⬆️
tests/test_remotes.py 98.42% <100%> (+0.01%) ⬆️
tests/test_initiate.py 100% <100%> (ø) ⬆️
src/hangar/constants.py 100% <100%> (ø) ⬆️
... and 33 more

@rlizzo rlizzo force-pushed the repo-creation branch 2 times, most recently from ae5f73d to 480acbe Compare August 4, 2019 11:09
@rlizzo rlizzo merged commit 137e473 into tensorwerk:master Aug 4, 2019
@@ -68,6 +89,7 @@ def __init__(self, base_path: os.PathLike, labelenv: lmdb.Environment,
commit_hash=self._commit_hash,
branchenv=self._branchenv,
refenv=self._refenv)
atexit.register(self.close)
Copy link
Member

Choose a reason for hiding this comment

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

IMO we need another function that gets called on atexit which does two things

  1. Printout a warning to the terminal saying what hangar did internally (closed the checkout for user) and it is not at all recommended. This is important because otherwise user would never remember to add the explicit close
  2. self.close()

Copy link
Member

Choose a reason for hiding this comment

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

@rlizzo Also, this raises warning if atexit has an unregistered function and the repo has been deleted before the exit.

@rlizzo rlizzo deleted the repo-creation branch August 7, 2019 16:49
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

2 participants