AccelerationConsortium / ac-microcourses

Microcourses hosted by the Acceleration Consortium for self-driving lab topics.
https://ac-microcourses.readthedocs.io/
MIT License
24 stars 3 forks source link

VS Code 4.2 assignment #122

Open sgbaird opened 4 days ago

sgbaird commented 4 days ago

I think there's something wrong with the micropico portion of the assignment (I don't think this package exists and also see point below), and I'm not sure that the mika(?) package is necessary. At least, I'm not sure when I would use it.

I think for the extensions, the focus could instead be on using it from a pure outcome standpoint. We may want to drop MicroPico from the assignment entirely, since I'm not sure there's a lot that can be done without the connection to a physical Pico W (which I don't think is worth trying to implement - too complicated). We should still mention it in the tutorial.

@RubainaFarin31 brought up the issue.

SissiFeng commented 1 day ago

redesigned this assignment https://github.com/ACC-SoftwareDev/4.2-VS-Code

sgbaird commented 1 day ago

Thanks for implementing! I was able to review this, and I have some comments below.

dataclass

While it's a useful construct, I think many learners will not be familiar with this at first. Because of that, I think it would need to be explained; however, I'd prefer to show people how to use pydantic instead. Thoughts? This could stay within 4.2 if we add some focus on the compatibility with VS Code since it uses typehints.

Black formatting

https://github.com/ACC-SoftwareDev/4.2-VS-Code/blob/7065552e6cad3c3cd150fc11b5a8981006b6054e/test_student_project.py#L24-L33

https://github.com/ACC-SoftwareDev/4.2-VS-Code/blob/7065552e6cad3c3cd150fc11b5a8981006b6054e/student_project.py#L16-L36

If the intention is to use black via its Python API, we should instead ask people to use it in the way they're more likely to in practice via the extension.

I suggest creating a separate Python file at the top-level with "messy" Python code as a regular script, and ask people to apply black formatting to it (however they want). Then, on the testing side, we use the black Python API to double check its consistency.

Type hints

It seems like the type hints are already implemented. I think that's OK; however, asking for TODO: Implement the function to calculate average and scaled values of numbers. seems irrelevant to the learning outcome here, which is to learn how to use type hint features within VS Code. Instead, I suggest instructing the user to copy-paste the "hover" type hint that appears to a user into a .txt file. Then, use pytest to check if the text matches what's expected. The "expected text" could be either hard-coded or programmatic (not sure if the latter is easy). For this one, I'm not too worried about hard-coding the answer.

devcontainer

Rather than giving learners a codespace with all required extensions installed by default, I think we should ask them to install the extensions themselves. Likewise for any VS Code settings they would be changing from the default settings, I think we should instead ask them to change the settings themselves per the instructions in https://code.visualstudio.com/docs/getstarted/settings. We can provide screenshots if needed.

docstring

Learners should use the extension, typing out """ to get the autocomplete to trigger and pressing "enter". Then, learners can be instructed to replace some of the placeholder text with some given text.

Ideally, this would be with numpy docstrings (personal preference), which users would need to configure via the corresponding setting.

SissiFeng commented 1 day ago

Some modifications based on comment above:

Dataclass → Pydantic: Replaced Person dataclass with Pydantic models in models.py Black Formatting: Changed to use VS Code's Black extension instead of Python API Type Hints: Focused on VS Code tooltips exploration rather than implementation Devcontainer: Removed pre-installed extensions to let students install manually Docstring: Added numpy-style docstring generation using VS Code extension

https://github.com/ACC-SoftwareDev/4.2-VS-Code

sgbaird commented 1 day ago

Nice! It looks like these already have the solutions embedded within it though? https://github.com/ACC-SoftwareDev/4.2-VS-Code/blob/main/docstring_practice.py

Likewise, https://github.com/ACC-SoftwareDev/4.2-VS-Code/blob/main/messy_code.py looks like it's already been formatted.

The tests look good to me.

Thank you!