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

πŸ› BUG BASH πŸ› Add: initial new tutorial - what is a Python package + images ✨ #117

Closed
wants to merge 17 commits into from

Conversation

lwasser
Copy link
Member

@lwasser lwasser commented Nov 30, 2023

This is the first in a set of tutorials on creating a Python package! it will have some broken links because all of the tutorials aren't added to this pr. This pr is setup to be used for the initial bug bash to be held dec 4-10 2023.

All reviews are welcome on this pr!


@lwasser lwasser changed the title Add: initial new tutorial - what is a Python package + images ✨ πŸ› BUG BASH πŸ› Add: initial new tutorial - what is a Python package + images ✨ Nov 30, 2023
Copy link
Collaborator

@ucodery ucodery left a comment

Choose a reason for hiding this comment

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

for packaging-lifecycle.png:
It is messing with me a bit that the Continuous Integration arrow is going in the opposite direction of the inner flow.
Also, I think that the numbering is unnecessary. I don't think it is wrong to start a project by writing the documentation, but it's never really talked about as a first step in these guides.
And it seems two documentations can be combined (still specifying user and contributor). That would leave a fifth spot open for Infrastructure (or is it File Structure, I'm still not sure).

- Documentation also helps people to contribute to your package.
- **Tests:** that makes sure your code works as it should and makes it easier for you and others to contribute to, modify and update the code in the future
- **License:** An open source license …. - link to choose a license…
- **Infrastructure** that automates updates, publication workflows and runs test suites
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the first mention of Infrastructure. The introduction and the accompanying image both state File Structure is the fifth element.

Copy link
Member Author

@lwasser lwasser Dec 22, 2023

Choose a reason for hiding this comment

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

great suggestion! i see the inconsistency.

let's explore this - are there 5 elements and what should they be?

  1. docs - contributor / development + user
  2. code
  3. tests (also code but keeping it separate as it's often something scientists add after even tho test based design is a thing)
  4. License
  5. Infrastructure (rather than file structure) - we could argue that all of the above things need to be organized in a specific way for a user to build & publish their package successfully.

Infrastructure can including

  • nox, make, testing infrastructure (tools, test runners tox/nox), CI, pre-commit, github as a platform or gitlab
    then there are people both inside maintainining and outside user / contributing.

tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
@lwasser
Copy link
Member Author

lwasser commented Dec 6, 2023

@ucodery thank you for being the first contributor to this lesson! yahoo! i'm going to let everyone leave comments before replying / fixing things.

All - you are more than welcome to "nit" on grammar and text. don't be shy about copy editing or just editing the text. I welcome all types of feedback. Thank you ALL for contributing to these tutorials and for helping me make them better!!

Copy link
Member Author

Choose a reason for hiding this comment

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

Note from @ucodery switch the arrow direction for CI πŸ˜† it's backwards!

Copy link
Contributor

@kierisi kierisi left a comment

Choose a reason for hiding this comment

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

small nits on the images (all things I'm happy to help with, just assign to me in asana!):

  • it would be great to get the text centered within the shapes
  • it'd be nice to have a consistent color palette. we're a bit all over the place between the images

Copy link
Contributor

@kierisi kierisi left a comment

Choose a reason for hiding this comment

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

added some notes! will pick back up again tomorrow :)

index.md Outdated Show resolved Hide resolved
index.md Outdated Show resolved Hide resolved
index.md Outdated Show resolved Hide resolved
index.md Outdated Show resolved Hide resolved
index.md Outdated Show resolved Hide resolved
index.md Outdated Show resolved Hide resolved
index.md Outdated Show resolved Hide resolved
index.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
Copy link
Contributor

@billbrod billbrod left a comment

Choose a reason for hiding this comment

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

I'm excited to see this! I've often tried to explain this to people, so will be nice to have a good resource.

I added some small comments where I think things can be clarified, as well as some small nitpicky formatting suggestions (being consistent with capitalization and monospace fonts for package names)

I also have a bigger question: is the term "package" interchangeable with "library"? If yes, should probably say so. If not, what's the difference? Quick survey: numpy describes itself as a "package" whereas matplotlib and seaborn use "library".

tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
Comment on lines 228 to 233
`matplotlib` does
one (big important) thing really well:

*It creates visual plots of data.*

`Matplotlib` is used by thousands of users for different plotting applications
Copy link
Contributor

Choose a reason for hiding this comment

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

matplotlib starts both these sentences, but is capitalized in only one of them. Should be consistent.

tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
:::{figure-md} build-workflow-tutorial
<img src="../images/tutorials/publish-package-pypi-conda.png" alt="Graphic showing the high level packaging workflow. On the left you see a graphic with code, metadata and tests in it. those items all go into your package. Documentation and data are below that box because they aren't normally published in your packaging wheel distribution. an arrow to the right takes you to a build distribution files box. that box leads you to either publishing to testPyPI or the real pypi. from PyPI you can then connect to conda forge for an automated build that sends distributions from PyPI to conda-forge." width="700px">

Once you are ready to make your code public, you can publish it on PyPI. Once the code is on PyPI it's a simple set of steps to publish to conda forge. You create a recipe using the grayskull package and then you open a pr in the conda-forge recipe repo. You will learn more about this process in the [conda-forge lesson](#).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Once you are ready to make your code public, you can publish it on PyPI. Once the code is on PyPI it's a simple set of steps to publish to conda forge. You create a recipe using the grayskull package and then you open a pr in the conda-forge recipe repo. You will learn more about this process in the [conda-forge lesson](#).
Once you are ready to make your code public, you can publish it on PyPI. Once the code is on PyPI it's a simple set of steps to publish to conda forge. You create a recipe using the `grayskull` package and then you open a pr in the conda-forge recipe repo. You will learn more about this process in the [conda-forge lesson](#).

Copy link
Member Author

Choose a reason for hiding this comment

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

i will come back to this as i think we need consensus around package names being within code brackets (monospace/ highlighted) vs not. i will open an issue about style guide items that we can track. as once we decide it's not hard to do find and replaces across the entire guide. and i think there are even tools to help with style guide enforcement that we could add to ci.

Copy link
Contributor

Choose a reason for hiding this comment

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

As an aside: I'd be very interested in learning more about style guide enforcement tools! Would a discussion of useful CI tools for python packaging tools belong in pyOpenSci? Asking for myself, because I'd love to have a list of them.

@lwasser
Copy link
Member Author

lwasser commented Dec 8, 2023

@all-contributors please add @ucodery for tutorials

Copy link
Contributor

@lwasser

I've put up a pull request to add @ucodery! πŸŽ‰

Copy link
Contributor

@NickleDave NickleDave 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 looking really good--I would like to teach from it πŸ™‚
Just added some suggestions, hope it helps

Comment on lines 107 to 109
A package is installable which means that you can add the functionality within
the package code to any Python environment and import that functionality like
you would import numpy or matplotlib.
Copy link
Contributor

Choose a reason for hiding this comment

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

In line with @billbrod's comment, how about rephrasing to something like

Suggested change
A package is installable which means that you can add the functionality within
the package code to any Python environment and import that functionality like
you would import numpy or matplotlib.
Once you make a package, you can install it in any environment and use its functionality by importing it just like you would import numpy or matplotlib

tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
tutorials/intro.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

A tiny nit for images/tutorials/packaging-101-outline.png:
Some subtitles (e.g., β€œLearn about…”, "Learn how...") have a period at the end, while others don’t. I recommend removing periods in all instances.

Copy link
Member Author

Choose a reason for hiding this comment

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

oops thank you @InessaPawson i missed this comment. i'll fix it in the newly opened pr!! my apologies for missing this. it's so easy to miss comments when there are a lot of them!!

@lwasser
Copy link
Member Author

lwasser commented Dec 12, 2023

@all-contributors please add @InessaPawson for code, review, tutorials

Copy link
Contributor

@lwasser

This project's configuration file has malformed JSON: .all-contributorsrc. Error:: Unexpected string in JSON at position 6816

@lwasser
Copy link
Member Author

lwasser commented Dec 12, 2023

hey everyone πŸ‘‹ i just wanted to THANK YOU all for the feedback here. Because some of you are newer to our review process here are the next steps

  1. Everyone here should have been added as a tutorial contributor to the guide. yes our friendly contributor bot has a tutorial category!! so even if you've contributed before this is a new contribution type that will be acknowledged on the website and in these lessons
  2. I am going to go back through all of the comments and rework the page. you will see comments from me as i implement each fix and sometimes a question from me to clarify what you are thinking in terms of how a change should be implemented
  3. Once that is all done i'll ping you all here again and will ask for another review to see if i've captured changes. If i don't hear back from you then, i'll assume all is good! :) no pressure to reply every but i appreciate feedback!

Then we merge once everyone is happen. NOTE: all packaging content is living content. Thus if we merge and need to update later we can do so by way of issues / pull requests as it makes sense. More from me soon!

lwasser and others added 9 commits December 21, 2023 18:53
Co-authored-by: William F. Broderick <[email protected]>
Co-authored-by: Jesse Mostipak <[email protected]>
Co-authored-by: Jesse Mostipak <[email protected]>
Co-authored-by: David Nicholson <[email protected]>
Co-authored-by: William F. Broderick <[email protected]>
@lwasser
Copy link
Member Author

lwasser commented Dec 22, 2023

@all-contributors please add @billbrod for tutorials

Copy link
Contributor

@lwasser

@billbrod already contributed before to tutorial

@lwasser
Copy link
Member Author

lwasser commented Dec 22, 2023

ok y'all - i've now created a shiny new / clean PR here: #139 with all of the updates from this round of reviews!! I welcome feedback on the new PR by 2 Jan 2023!! let's try to merge it by the 3rd.

@lwasser
Copy link
Member Author

lwasser commented Dec 22, 2023

closing this given the new pr is open now!

@lwasser lwasser closed this Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants