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

Feedback fixes #25

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Feedback fixes #25

wants to merge 4 commits into from

Conversation

christopher-wild
Copy link
Owner

@christopher-wild christopher-wild commented Aug 14, 2024

addresses feedback for episodes 2, 3 & 4

closes #17 closes #18 closes #19

Copy link

github-actions bot commented Aug 14, 2024

Thank you!

Thank you for your pull request 😃

🤖 This automated message can help you check the rendered files in your submission for clarity. If you have any questions, please feel free to open an issue in {sandpaper}.

If you have files that automatically render output (e.g. R Markdown), then you should check for the following:

  • 🎯 correct output
  • 🖼️ correct figures
  • ❓ new warnings
  • ‼️ new errors

Rendered Changes

🔍 Inspect the changes: https://github.com/christopher-wild/FAIR4RS-Packaging/compare/md-outputs..md-outputs-PR-25

The following changes were observed in the rendered markdown documents:

 accessing-packages.md   | 39 +++++++++++++++++--------
 creating-packages.md    | 26 ++++++++---------
 md5sum.txt              | 34 +++++++++++-----------
 package-file-history.md | 75 +++++++++++++------------------------------------
 4 files changed, 77 insertions(+), 97 deletions(-)
What does this mean?

If you have source files that require output and figures to be generated (e.g. R Markdown), then it is important to make sure the generated figures and output are reproducible.

This output provides a way for you to inspect the output in a diff-friendly manner so that it's easy to see the changes that occur due to new software versions or randomisation.

⏱️ Updated at 2024-09-30 10:05:51 +0000

github-actions bot pushed a commit that referenced this pull request Aug 14, 2024
Comment on lines 130 to 134

::: keypoints
- pyproject.toml files are the current recommended packaging file for Python
- other standards are still used and you may come across them
:::
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the key points here can be better summarised that align with the episode's questions/learning objectives

::: callout
You may notice a wheel file download during the pip install, for example `Downloading numpy-2.0.0-cp312-cp312-win_amd64.whl`. A wheel in Python is a prebuilt package format that allows for quicker and more efficient installation, so when it is downloaded your local computer doesn't need to do any building. The alternative is source files which often take the form `.zip` or `.tar.gz`, which when downloaded will then need to be built then installed, which is often far slower.

:::

::: challenge
### Exercise 2: Create venv and install Numpy
Copy link
Collaborator

@f-allian f-allian Sep 17, 2024

Choose a reason for hiding this comment

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

The Windows/Mac tabs look absolutely massive here and in another one below! Might be because of the number of #?

image

Copy link
Owner Author

Choose a reason for hiding this comment

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

They do look commically big on yours! The amount of #'s is the same as the workbench recommends and looks okay on mine
image.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@christopher-wild that's strange! Never mind then!


::: instructor

The above command should be universal on both windows and mac/unix setups. It may be worth checking with the class at this point that they are all familiar with the -m notation, and what the above command does exactly
It may be worth checking with the class at this point that they are all familiar with the -m notation, and what the above command does exactly

:::

Copy link
Collaborator

Choose a reason for hiding this comment

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

Again I think the key points below can be made more informative

Copy link
Collaborator

@f-allian f-allian left a comment

Choose a reason for hiding this comment

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

@christopher-wild Sorry for the delay on this. I've left a few suggestions below.

@christopher-wild
Copy link
Owner Author

@christopher-wild Sorry for the delay on this. I've left a few suggestions below.

Cheers for the helpful feedback, let me know if you think the new keypoints are any better 🙂

github-actions bot pushed a commit that referenced this pull request Sep 30, 2024
Copy link
Collaborator

@f-allian f-allian left a comment

Choose a reason for hiding this comment

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

Looks great thanks @christopher-wild!

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.

Feedback: Section 4: Create package Feedback: Section 3: Accessing Packages Feedback: Section 2. History!
2 participants