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

feat(activity): add activity 2 #17

Merged
merged 6 commits into from
Oct 11, 2024
Merged

Conversation

lwasser
Copy link
Member

@lwasser lwasser commented Oct 8, 2024

This is the second activity in the workshop. It's complete short of the answers to the OYO sections. i have an answer key script as well that i can push separately.

@lwasser
Copy link
Member Author

lwasser commented Oct 8, 2024

NOTE: the CI issues will be fixed when some of the other pr's are merged. I'm worrying less about that now and more about content!!

clean-modular-code/activity-2/clean-code-activity-2.md Outdated Show resolved Hide resolved
@@ -36,15 +51,19 @@ What tools could make the code more DRY?

## Reproducibility: will this code run on other machines?

One big challenge when considering the reproducibility of workflows is ensuring that your code runs seamlessly on different machines or environments. One key issue is file paths. Hard-coded paths, like "data/part-1-data.json", can lead to errors if the directory structure on another machine differs from your setup. Paths should be constructed dynamically, using tools like Python’s pathlib or environment variables to ensure flexibility across different systems.
One big challenge when considering the reproducibility of workflows is ensuring that your code runs seamlessly on different machines or environments. One key issue is file paths. Hard-coded paths, like `data/part-1-data.json`, can lead to errors if the directory structure on another machine differs from your setup. Paths should be constructed dynamically, using tools like Python’s `pathlib` or environment variables to ensure flexibility across different systems.
Copy link
Contributor

Choose a reason for hiding this comment

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

I definitely thought you were leading up to Window vs POSIX path slashes. I think that is a more basic, and easier to automate, difference between environments. If the directory structure actually changes in new environments, I think there is only so much you can do without needing user input.

Copy link
Member Author

@lwasser lwasser Oct 11, 2024

Choose a reason for hiding this comment

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

Oh - @ucodery i didn't understand this comment at first but now i think i do.

this section IS about windows vs POSIX path slashes. My language that says "directory structure on another machine differs" is confusing/incorrect!!

As you point out, that is a much harder problem that we can't solve with code. How about I edit this paragraph to make it clearer? It's fully about path creation, which we can handle with code.

@lwasser
Copy link
Member Author

lwasser commented Oct 11, 2024

Cool - CI is failing with things are are fixed in #18 #13 i am going to merge this soon after one last round of revisions and adding answers to the OYOs

Copy link
Contributor

@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.

I just read this one section, I couldn't easily tell if anything else has changed

clean-modular-code/activity-2/clean-code-activity-2.md Outdated Show resolved Hide resolved
@lwasser
Copy link
Member Author

lwasser commented Oct 11, 2024

thank you, @ucodery. If you want to have a look at a preview of the published (ci) page - it's here - https://output.circle-artifacts.com/output/job/b18a4bf1-8f83-4764-a52b-e0e61a07f909/artifacts/0/html/clean-modular-code/activity-2/clean-code-activity-2.html I've made small edits and clarified things which is why it may be hard to identify what has changed.

The big change here is that I rewrote the POSIX path section and added examples. I'm hopeful that makes it more clear! Thank you for the review comments - things were not worded well before.

@lwasser
Copy link
Member Author

lwasser commented Oct 11, 2024

I am going to merge this PR, rebase and focus more time on activity 3 next week. Thank you again @ucodery for all of the input!! the CI issues are related to other lessons not in this PR. They will all get fixed once things are merged! Lesson dev is iterative as we focus on the content and move things around accordingly.

@lwasser lwasser merged commit 59736ca into pyOpenSci:main Oct 11, 2024
1 of 2 checks passed
@lwasser lwasser deleted the activity-2 branch October 11, 2024 23: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.

2 participants