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

Apply Pod 02 W2's feedback #2

Merged
merged 8 commits into from
Oct 12, 2022
Merged

Apply Pod 02 W2's feedback #2

merged 8 commits into from
Oct 12, 2022

Conversation

MrCordeiro
Copy link
Collaborator

  • Split Assignment to be run on its own week
  • Add missing .gitignore file
  • Add Anaconda instructions
  • Introduce initial project structure and explain what each directory is for

@MrCordeiro MrCordeiro added the documentation Improvements or additions to documentation label Oct 7, 2022
@MrCordeiro MrCordeiro requested a review from hershaw October 7, 2022 16:50
@MrCordeiro MrCordeiro self-assigned this Oct 7, 2022
@MrCordeiro MrCordeiro changed the title Dda/po2 w2 feedback P02 W2 feedback Oct 7, 2022
@MrCordeiro MrCordeiro changed the title P02 W2 feedback Pod 02 W2 feedback Oct 7, 2022
@MrCordeiro MrCordeiro changed the title Pod 02 W2 feedback Apply Pod 02 W2's feedback Oct 7, 2022
Copy link
Contributor

@hershaw hershaw left a comment

Choose a reason for hiding this comment

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

Only thing I'd consider is taking a hard line on virtual envs and not including support for anaconda. If we support anaconda at the beginning then we have to support it for everything.

@@ -82,34 +82,37 @@ Week 02 _(~1.5 hours)_

- Linting
- Continuous integration

Week 03 _(~2.5 hours)_
Copy link
Contributor

Choose a reason for hiding this comment

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

All good. We should ping Albina that it has been extended by a week because the first weeks were a bit too heavy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just a clarification here: at this point, I'm evolving the Daredata branch separately from the NOS branch as it was dawning on me that their constraints are different. The NOS repo, in fact, has 10 weeks and not 9 (see: nosportugal/faast-foundations#1). I got pushback whenever a unit went beyond 3 weekly hours over there. On the other hand, our pods are super motivated and will do the courses on nights and weekends if they need to.

Another change exclusive to the NOS repo is that I switched the Type Hint content for something more basic and easier to digest.

PS: I already gave Albina a heads up that the week suggestion might change depending on the feedback of the first pods 😉


- Design patterns
- Packaging and publishing

Week 08 _(~2 hours)_
Week 09 _(~2 hours)_
Copy link
Contributor

Choose a reason for hiding this comment

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

9 weeks is so long. I'm thinking that we could add another calendar that is an accelerated one for people that have 10-15 hours per week for study. Might be nice to let people know that it can be done in 3 or 4 weeks if you've got more time. I think this might be worth it for the Alfas so maybe we bring it up with Diogo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since this repo is growing on a separate track from NOS, we can reunify some weeks to make it fit into an 8-week schedule again. Do you have any suggestions?

2. Create a virtual environment with `python -m venv .venv`
3. Activate the virtual environment with `source .venv/bin/activate` or `.venv\Scripts\activate` on Windows
4. Install its dependencies on editable mode with `pip install -e .[dev]`.
2. Create a virtual environment with `python -m venv .venv`. If you are using conda, you can create a virtual environment with `conda create --name foundations`.
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to have an anaconda option? My feeling is that virtual environments should be required...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah... Anaconda was screwing things up for one of the mentees; at this point, they didn't have the unit on "virtual environments" yet and they are still too green to fully understand how Anaconda was messing with their settings.

The problem was that even if they had python already installed - and even if you make VSCode use that interpreter instead of Anaconda's, Anaconda just overrides their bash profile and runs its own environments.

It's a mess: if you follow the commands directly, Anaconda just creates a venv inside their venv. Suppose you try to deactivate the Anaconda venv. In that case, you either generate another venv inside their "base" venv or you can't find python anymore since Anaconda overrode your PYTHON_PATH when the terminal started.

Me adding Anaconda's commands was a sign of me giving up solving this properly and just trying to unblock the mentee. The mentoring hour would go way beyond the allotted time otherwise.

@MrCordeiro
Copy link
Collaborator Author

@hershaw, did anyone from your pods have issues with venv/Anaconda yet?

assignments/assignment_1/README.md Outdated Show resolved Hide resolved
assignments/README.md Outdated Show resolved Hide resolved
assignments/README.md Outdated Show resolved Hide resolved
Reinforce that Anaconda users should try to create venv with the canonical python method first.
Reinforce that Anaconda users should try to create venv with the canonical python method first.
@MrCordeiro MrCordeiro merged commit 8917056 into main Oct 12, 2022
@MrCordeiro MrCordeiro deleted the dda/po2_w2_feedback branch October 12, 2022 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants