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

Nbless: Software Submission for Review #7

Closed
11 of 22 tasks
marskar opened this issue May 30, 2019 · 30 comments
Closed
11 of 22 tasks

Nbless: Software Submission for Review #7

marskar opened this issue May 30, 2019 · 30 comments
Labels
6/pyOS-approved archived Package is marked as archived

Comments

@marskar
Copy link

marskar commented May 30, 2019

Submitting Author: Martin Skarzynski (@marskar)
All current maintainers: (@marskar)
Package Name: Nbless
One-Line Description of Package: Construct, deconstruct, convert, execute, & prepare slides from Jupyter notebooks
Repository Link: https://github.com/py4ds/nbless
Version submitted: 0.2.26
Editor: @lwasser
Reviewer 1: @ocefpaf
Reviewer 2: @kysolvik
Archive: N/A
JOSS DOI: N/A
Version accepted: TBD
Date accepted (month/day/year): 06/17/2019


Description

  • Include a brief paragraph describing what your package does:

    The Nbless Python package allows you to
    - (de)construct,
    - convert,
    - execute, and
    - prepare slides from
    Jupyter notebooks.

    The Nbless Python package consists of 6 Python functions and shell commands:
    - nbconv, which converts a notebook into various formats.
    - nbdeck, which prepares a notebook to be viewed as or converted into slides.
    - nbexec, which runs a notebook from top to bottom and saves an executed version.
    - nbless, which calls nbuild and nbexec to create and execute a notebook.
    - nbraze, which extracts code and markdown files from a notebook.
    - nbuild, which creates a notebook from code and markdown files.

    For more on Nbless, please take a look at the official documentation.

Scope

  • Please indicate which category or categories this package falls under:
    • Data retrieval
    • Data extraction
    • Data munging
    • Data deposition
    • Reproducibility
    • Geospatial
    • Education
    • Data visualization*

* Please fill out a pre-submission inquiry before submitting a data visualization package. For more info, see this section of our guidebook.

  • Explain how and why the package falls under these categories (briefly, 1-2 sentences):

    Jupyter notebooks are important for reproducibility, because they facilitate literate programming, the combination of code and text.
    Literate programming is advantageous, because it allows descriptions of methods and results to be adjacent to the code used to obtain the results.
    I frequently use Jupyter notebooks in my work and in the Python classes I teach, but I often wish I could share small, simple files instead of large Jupyter notebooks.
    With Nbless, anyone can create Jupyter notebooks from simple source files or extract code and markdown files from Jupyter notebooks.
    In this way, Nbless supports round-trip conversion between source files and Jupyter notebooks. In other words, code and markdown files can be extracted from a Jupyter notebook AND then used to recreate the original Jupyter notebook.
    This can promote the modularity principle without sacrificing the benefits of literate programming or the convenience of a single file with all code, text, and output for the project.
    Code and markdown files can be easily be accessed with any text editor or even opened in a browser without an internet connection. Keeping source files makes version control simpler than trying to track changes made to Jupyter notebooks (see nbdime for another solution). Code files (.py, .R, etc.) are amenable to testing and linting, while notebooks are difficult to lint/test (see autopep8 and nbval).

  • Who is the target audience and what are scientific applications of this package?

    The target audience is anyone who uses Jupyter notebooks. This package may help teachers and researchers to improve the efficiency of their Jupyter notebook workflows. One of the major features of Nbless is that it can facilitate the creation of slides from Jupyter notebooks. Jupyter slides can be converted in revealjs HTML files or viewed using nbviewer or RISE/Binder.
    I can imagine teachers and researchers writing markdown and code files and then using nbless to creating Jupyter notebooks to share with their students and colleagues.

  • Are there other Python packages that accomplish the same thing? If so, how does yours differ?

    Related packages include:

    These packages can all convert Jupyter notebooks to other formats: markdown files (all three) or Python scripts (jupytext). Nbless wraps nbconvert to convert between file types, but it can do something all of the aforementioned packages cannot. Nbless can take a more modular approach to file conversion by extracting the contents of each notebook cell into a separate file (cell -> file) or using a source file to create each notebook cell (file -> cell). Looking beyond simple file conversion, Nbless includes a tool for making slides from notebooks (by setting slide_type in notebook metadata).

  • If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted:

    The pre-submission inquiry link is Nbless: (de)construct, convert, execute, and prepare slides from Jupyter notebooks. #4

    I will close Nbless: (de)construct, convert, execute, and prepare slides from Jupyter notebooks. #4 as soon as I am done with Nbless: Software Submission for Review #7.

Technical checks

For details about the pyOpenSci packaging requirements, see our packaging guide. Confirm each of the following by checking the box. This package:

  • does not violate the Terms of Service of any service it interacts with.
  • has an OSI approved license
  • contains a README with instructions for installing the development version.
  • includes documentation with examples for all functions.
  • contains a vignette with examples of its essential functions and uses.
  • has a test suite.
  • has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.

Publication options

JOSS Checks
  • The package has an obvious research application according to JOSS's definition in their submission requirements. Be aware that completing the pyOpenSci review process does not guarantee acceptance to JOSS. Be sure to read their submission requirements (linked above) if you are interested in submitting to JOSS.
  • The package is not a "minor utility" as defined by JOSS's submission requirements: "Minor ‘utility’ packages, including ‘thin’ API clients, are not acceptable." pyOpenSci welcomes these packages under "Data Retrieval", but JOSS has slightly different criteria.
  • The package contains a paper.md matching JOSS's requirements with a high-level description in the package root or in inst/.
  • The package is deposited in a long-term repository with the DOI:

Note: Do not submit your package separately to JOSS

Are you OK with Reviewers Submitting Issues to your Repo Directly?

This option will allow reviewers to open smaller issues that can then be linked to PR's rather than submitting a more dense text based review. It will also allow you to demonstrate addressing the issue via PR links.

  • Yes I am OK with reviewers submitting requested changes as issues to my repo. Reviewers will then link to the issues in their submitted review.

Code of conduct

P.S. Have feedback/comments about our review process? Leave a comment here

Previous Repo: https://github.com/marskar/nbless/

@lwasser
Copy link
Member

lwasser commented May 30, 2019

Editor checks:

  • Fit: The package meets criteria for fit and overlap.
  • Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service. TRAVIS
  • [x ] License: The package has an OSI accepted license MIT
  • Repository: The repository link resolves correctly
  • Archive (JOSS only, may be post-review): The repository DOI resolves correctly
  • Version (JOSS only, may be post-review): Does the release version given match the GitHub release (v1.0.0)?

Editor comments

Thank you for this submission @marskar @ocefpaf and @leouieda will begin their review
Note: Martin is open to reviewers submitting issues to the repo directly. So please as you create your review, link to those issues if you create them

Reviewers: @ocefpaf @leouieda
Due date: 24 June 2019

@ocefpaf i'm suggesting a review deadline of June 24. It sounded like you wanted to do reviews one at a time. So @ocefpaf will submit his first and @leouieda ... will the two of you please coordinate on that? this is a new process so let's figure out what works best. also new is submitting issues directly rather than responding to a lot of text. A link to the reviewer template is below. I was thinking about adding readme checks to the reviewer template. please let me know what you think about that. i think it might make things easier!

Editor and review templates can be found here

@ocefpaf
Copy link
Member

ocefpaf commented Jun 2, 2019

@lwasser, @leouieda and @marskar here is my review.
@marskar please feel free to ignore the checked items when copying this to answer and use only the unchecked ones. Also, every actionable item has an issue to the package repository linked, so feel free to answer there first and then summarize/cross-reference it here.

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s) demonstrating major functionality that runs successfully locally
  • Function Documentation: for all user-facing functions
  • Examples for all user-facing functions
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer.

My suggestion is to reduce the overlap of the README with the documentation and make it more to the point: install and use examples (py4ds/nbless#16). There is one section in the docs that is quite confusioning and should probably be just an example of a workflow instead, with a link to the file (py4ds/nbless#15). The package reports two different version numbers (py4ds/nbless#12).

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.

I found two minor problems when trying to run the CLI as the docs recommend:

Test covers only 2 out of 6 main functions (py4ds/nbless#17).

  • Continuous Integration: Has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.
  • Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.

Review Comments

The package is a good fit for PyOpenSci. The documentation could use some polishing, the CLI examples are good but the rest is a bit terse to read. I believe that one of the functionality issues I found may be due to documentation problems and not really a bug (py4ds/nbless#13).

The repository does not have a community guidelines. (Note to PyOpenSci: do we have examples of those to point people to?)

Both development and standard installations were easy and all the tests passed. My doubt is regarding the pandoc dependency declared on the CI. Pandoc is optional in nbconvert but I know that some results may be different if you have it present. Does nbless expect pandoc for some features? if so are those tested properly? The tests passes without it BTW but I found this comment and I could not find a test against the expected output.

@marskar
Copy link
Author

marskar commented Jun 2, 2019

Thanks for the very helpful review @ocefpaf!
I plan to work through each of the 6 issues before the end of the week.

In the meantime, the answer to your question regarding pandoc is that the nbconv function/command (a wrapper for jupyter nbconvert) requires pandoc for conversion to all formats other than HTML. Converting to PDF also requires LaTeX. I will make sure to include this in the docs.
The nbconv tests that you linked to are commented out, because I had trouble satisfying the pandoc dependency in my Travis build. I will try to figure out how to get pandoc and Travis to cooperate.

@marskar
Copy link
Author

marskar commented Jun 17, 2019

@ocefpaf
Copy link
Member

ocefpaf commented Jun 19, 2019

@lwasser I believe that @marskar addressed all the issues I raised in my review. (I updated the check boxes in #7 (comment) to reflect that.)

@lwasser
Copy link
Member

lwasser commented Jun 19, 2019

awesome. ok so we just need a second review now i think? we ran into an issue where we need reviewers for earthpy too.. and so i think @leouieda was asked twice which isn't ideal. Leo would you prefer to review this package or earthpy? we can try to find others for earthpy if need be!

and thank you both for working through this process - it seemed pretty seamless the way you implemented the review and changes.

@leouieda
Copy link
Member

Hi all, sorry I haven't been able to stay up to date with my github notifications. I don't know if I'll be able to get this done until the end of month.

@lwasser I don't have a preference but I feel like it would be easier to find someone else to review nbless than earthpy, since nbless is general purpose. I could do earthpy if you prefer.

@marskar
Copy link
Author

marskar commented Jun 20, 2019

Hi @lwasser, @ocefpaf, and @leouieda,
Indeed, the review was seamless and very helpful.
I made many improvements including getting the test coverage up to 100%.
The current nbless version (0.2.38) is ready for the second review, but no rush.
In the meantime, I would like to volunteer as a reviewer for earthpy if you think that would work.
I will ask for reviewers in the Python for Data Science (py4ds) community I run.
We can discuss this on the call tomorrow.

@lwasser
Copy link
Member

lwasser commented Jun 20, 2019

@marskar that would be wonderful. it sounds like you and @leouieda could do earthpy and then maybe @kysolvik or @carsonfarmer could be interested in reviewing nbless. i'll put your names in the doc and we can chat today. @luizirber note that we may have two reviewers for earthpy now!!

@lwasser
Copy link
Member

lwasser commented Jun 20, 2019

@kysolvik are you interested in reviewing this package? Leo is open to reviewing earthpy. please let me know!!

@kysolvik
Copy link
Contributor

@lwasser yep I'm happy to!

@kysolvik
Copy link
Contributor

Hi all! Sorry for the delay. 3 weeks passed fast. Just wanted to give @marskar a heads up that I started working on the review today. I'll try to post this afternoon or tomorrow.

@kysolvik
Copy link
Contributor

kysolvik commented Jul 12, 2019

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s) demonstrating major functionality that runs successfully locally

KS: Maybe consider moving the nbuild vignette before the nbless one? I was unclear on how nbless would build the notebook, but the nbuild section explains it.

  • Function Documentation: for all user-facing functions
  • Examples for all user-facing functions
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer.

KS: I didn't see these.

Readme requirements
The package meets the readme requirements below:

  • Package has a README.md file in the root directory.

The README should include, from top to bottom:
re

  • The package name
  • Badges for continuous integration and test coverage, the badge for pyOpenSci peer-review once it has started (see below), a repostatus.org badge, and any other badges. If the README has many more badges, you might want to consider using a table for badges, see this example, that one and that one. Such a table should be more wide than high.
  • Short description of goals of package, with descriptive links to all vignettes (rendered, i.e. readable, cf the documentation website section) unless the package is small and there’s only one vignette repeating the README.
  • Installation instructions
  • Any additional setup required (authentication tokens, etc)
  • Brief demonstration usage
  • Direction to more detailed documentation (e.g. your documentation files or website).
  • If applicable, how the package compares to other similar packages and/or how it relates to other packages

KS: In the related projects section, could include a blurb about how nbless is different.

  • Citation information

KS: General review notes on docs
Minor issue with the docs sidebar not being scrollable (py4ds/nbless#30).

I think the README is still a bit lengthy and dense, but it's not a big deal if it's hard to break up. Ideally
maybe the README could just have the nbconv section and then a list of the rest of the functionality with links to the docs for usage examples. But I totally leave this to you if you want to have all the commands in the README.

For nbexec, still some inconsistencies in docs (py4ds/nbless#13).

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests: Tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Continuous Integration: Has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.
  • Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 2-3


Review Comments

Awesome package @marskar! The CLI is super easy to use. I can imagine it being used frequently, especially for education. I opened a couple issues and added a couple comments, but they're all minor. I opened a few issues. Let me know if I can help at all!

Summary of the issues:

@lwasser
Copy link
Member

lwasser commented Jul 15, 2019

thank you @kysolvik !! @marskar it looks like you saw the review. please let us know if you have any questions or comments. when you are done addressing Kylen's comments above, please ping me!!

@kysolvik
Copy link
Contributor

Looks good to me @marskar and @lwasser!! Good work all, especially @marskar for creating an awesome package!

@lwasser
Copy link
Member

lwasser commented Jul 16, 2019

@marskar i suspect we have our FIRST PACKAGE accepted. let's get a final ok from @ocefpaf who i know is very busy today after his conference trip!!

@lwasser
Copy link
Member

lwasser commented Jul 16, 2019

oh housekeeping. @marskar would you prefer to move your package OVER HERE to our org. or keep it in your space and we list it on the website? the choice is yours.

@marskar
Copy link
Author

marskar commented Jul 16, 2019

I think it might be better to keep it where it is now, but I'd be happy to discuss this in our next community call.

@ocefpaf
Copy link
Member

ocefpaf commented Jul 17, 2019

Looks good to me @marskar and @lwasser!! Good work all, especially @marskar for creating an awesome package!

Looks good to me too! +1 to approve nbless!

@lwasser
Copy link
Member

lwasser commented Jul 17, 2019

yahoo!! ok i'll followup with a badge. i don't think anyone has respond to my discourse post yet. and i'll double check we aren't missing anything else that ROS does when accepting a package!

@lwasser
Copy link
Member

lwasser commented Aug 6, 2019

ok this package has been ACCEPTED into pyopensci. it lives here: https://github.com/marskar/nbless/ . we now need to put it somewhere on our website so there is a connection between it and pyopensci. i'll create a separate issue for that in our website repo and ill close this here!!

pyOpenSci/pyopensci.github.io#3

@lwasser lwasser closed this as completed Aug 6, 2019
@lwasser lwasser changed the title Nbless: Software Submission for Review Nbless: Software Submission for Review (APPROVED & ACCEPTED) Aug 6, 2019
@lwasser
Copy link
Member

lwasser commented Sep 15, 2022

hey 👋 @ocefpaf @kysolvik @marskar ! I hope that you are all well. I am reaching out here to all reviewers and maintainers about pyOpenSci now that i am working full time on the project (read more here). We have a survey that we'd like for you to fill out so we can:

🔗 HERE IS THE SURVEY LINK 🔗

  1. invite you to our slack channel to participate in our community (if you wish to join - no worries if that is not how you prefer to communicate / participate).
  2. Collect information from you about how we can improve our review process and also better serve maintainers.
    The survey should take about 10 minutes to complete depending upon how much you decide to write. This information will help us greatly as we make decisions about how pyOpenSci grows and serves the community. Thank you so much in advance for filling it out.

NOTE: this is different from the form designed for reviewers to sign up to review.
If there are other maintainers for this project, please ping them here and ask them to fill out the survey as well. It is important that we ensure packages are supported long term or sunsetted with sufficient communication to users. Thus we will check in with maintainers annually about maintenance.

Thank you in advance for doing this and supporting pyOpenSci.

@lwasser
Copy link
Member

lwasser commented Sep 28, 2022

hey there @kysolvik @marskar 👋 Just a friendly reminder to take 5-10 minutes to fill out our survey . We really appreciate it. Thank you in advance for helping us by filling out the survey!! 🙌

Martin, it's really important for us to collect information from our maintainers so that we can both stay in touch with you regarding package maintenance and also support you through time. We really appreciate your time in filling this out. Also are you the sole maintainer of this package? if not, please have your co-maintainers also fill it out and please list them here as well. Many thanks in advance!

✨ Filipe, thank you so much for taking the time to fill it out 🙌

🔗 HERE IS THE SURVEY LINK 🔗

@lwasser
Copy link
Member

lwasser commented Oct 19, 2022

hey there @kysolvik - I know you've done a lot for us already - but i'm hoping you can still fill out our survey just so we have a record of you in our review queu . The link is here: https://forms.gle/DF4qoAdXsCUuerj48 Many thanks for doing this i know time is valuable.

@lwasser
Copy link
Member

lwasser commented Oct 19, 2022

hey @marskar this is related to both the survey (which i'd greatly appreciate your filling out But also I noticed that this package hasn't been updated since 2019. I'm just wondering if this package is still maintained by you or someone? Of if we need to sunset / archive it . it has some stars but i haven't look at the stats to see if people are using it. Please reply here and let me know. it's important that packages in our ecosystem are maintained. And if they aren't we need to either find a new reviewer OR we need to help you sunset and archive it so people know it's not actively maintained.

Many thanks for responding. you can also email me if that is easier at leah at pyopensci.org
All the best.

@banesullivan
Copy link
Collaborator

I've add the archived label to this review

cc @lwasser

@lwasser lwasser changed the title Nbless: Software Submission for Review (APPROVED & ACCEPTED) Nbless: Software Submission for Review Dec 17, 2024
@lwasser
Copy link
Member

lwasser commented Dec 17, 2024

oh this reminds me @banesullivan we have a workflow that moves packages on our project board based on the label setting. we will want to add archive https://github.com/pyOpenSci/software-submission/blob/main/update-project.json to the json file so archived packages are moved too!

i started a doc to document this process. let's use this to document this and also the label workflow!!

@lwasser
Copy link
Member

lwasser commented Dec 17, 2024

also @marskar i hope that you are well and that perhaps you've just moved on to bigger and better things 💟

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6/pyOS-approved archived Package is marked as archived
Projects
Status: pyos-accepted
Development

No branches or pull requests

6 participants