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

Version Awareness #24

Merged
merged 8 commits into from
Jun 9, 2020
Merged

Version Awareness #24

merged 8 commits into from
Jun 9, 2020

Conversation

jayqi
Copy link
Member

@jayqi jayqi commented Jun 6, 2020

  • Version awareness for post-save hook installation. Resolves Check for existing config and don't process if not new #16
    • Add versioneer for version management
    • Add a --version flag to CLI. I copied this example from the Typer docs.
    • Post-Save Hook installation is version-aware:
      • Version is included in comment string
      • Detects existing nbautoexport post-save hook
      • Replaces existing block if current version is higher (including if there is no version)
  • Create all directories necessary for Jupyter config file if they don't already exist. Seems reasonable given that we create a blank config file if the directory exists but the file doesn't. Resolves How to handle missing Jupyter config directory #19.
  • Narrowed coverage to only report on source files in nbautoexport/, excluding _version.py from versioneer. This looks like:
Name                           Stmts   Miss  Cover
--------------------------------------------------
nbautoexport/__init__.py           5      0   100%
nbautoexport/nbautoexport.py     136     46    66%
--------------------------------------------------
TOTAL                            141     46    67%

Copy link
Member

@pjbull pjbull 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 awesome!

else:
c.FileContentsManager.post_save_hook = post_save
except:
pass
Copy link
Member

Choose a reason for hiding this comment

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

Now that this method is getting more complicated, let's see if there's a good way to store this block as Python code so it gets highlighting in source control and could be easier to test directly if we need to eventually. I think that we could either define the function and use inspect.getsource to get this code after the function is imported, or maybe just have it in a .py that we read instead of importing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Opened #26

nbautoexport/nbautoexport.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@r-b-g-b r-b-g-b left a comment

Choose a reason for hiding this comment

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

One thing to check -- looks great!

MANIFEST.in Outdated Show resolved Hide resolved
@@ -209,6 +255,9 @@ def install(
verbose: bool = typer.Option(
False, "--verbose", "-v", is_flag=True, show_default=True, help="Verbose mode"
),
version: bool = typer.Option(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kewl

@jayqi jayqi requested a review from r-b-g-b June 8, 2020 16:28
@jayqi
Copy link
Member Author

jayqi commented Jun 8, 2020

@r-b-g-b ready for re-review.

@r-b-g-b r-b-g-b merged commit af222d2 into master Jun 9, 2020
@r-b-g-b r-b-g-b deleted the 16-version-awareness branch June 9, 2020 18:41
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.

How to handle missing Jupyter config directory Check for existing config and don't process if not new
3 participants