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

Clean up python code #90

Open
ksuchak1990 opened this issue Apr 1, 2022 · 8 comments
Open

Clean up python code #90

ksuchak1990 opened this issue Apr 1, 2022 · 8 comments

Comments

@ksuchak1990
Copy link

I've had a quick flick through the python code for TDS, and think there's a couple of things that could be done to clean it up:

  • Making the code compliant with styleguide conventions; this will make code more easily readable and encourage good habits from students.
  • Adding a requirements.txt or conda environment for the required packages; there is currently an attached docker image but some students may not have experience with docker so this would make it easier for them to get started.

Very happy to pick this up myself as I think it's a pretty easy fix - let me know what you think.

@Robinlovelace
Copy link
Member

Robinlovelace commented Apr 1, 2022

Hi @ksuchak1990 I would massively welcome that. I'm a Pythong beginner and the Python code is inherited from @charles-fox who kindly let us port it into my teaching materials. It's a bit out of date and sure it could do with a bit of a spring clean at the very least 🚿 Many thanks 🚀

@DrPayne25
Copy link

Hi @ksuchak1990 my team and I would like to work on this request. We were thinking of adding a sample requirements.txt file and a how to setup docker markdown file with instructions. Anything else we would need to know before getting started on this?

@ksuchak1990
Copy link
Author

Hi @DrPayne25, that sounds good to me. I would check with @Robinlovelace too.

  • I think that having a "how to set up docker" is a good idea (@Robinlovelace - I don't know whether this exists somewhere else in the repo?).
  • With regards to the requirements.txt file, I think that a starting point would be to have one that included all of the packages from each of the scripts in the folder and subfolders. This way, users would be able to set up a development environment with all of the appropriate packages.

@Robinlovelace
Copy link
Member

Hi guys, the Docker containers are slightly out of date but should still work. I'm open minded and as I say a beginner here, the Python code is definitely out-of-date so updating it with more modern packages will be very welcome. There is a 'how to set-up docker', there was content in the README but I removed it as it was a bit much for students that year! https://github.com/ITSLeeds/TDS/tree/master/docker

@Robinlovelace
Copy link
Member

With regards to the requirements.txt file, I think that a starting point would be to have one that included all of the packages from each of the scripts in the folder and subfolders. This way, users would be able to set up a development environment with all of the appropriate packages.

Regarding requirements.txt my understanding is that environment.yml files with instructions for conda are generally preferred now, no? If so let's switch to the generally accepted best practice, although I know there are active debates on that! https://twitter.com/search?q=%22don%27t%20use%22%20%20python%20&src=typed_query&f=top

@ksuchak1990
Copy link
Author

With regards to the requirements.txt file, I think that a starting point would be to have one that included all of the packages from each of the scripts in the folder and subfolders. This way, users would be able to set up a development environment with all of the appropriate packages.

Regarding requirements.txt my understanding is that environment.yml files with instructions for conda are generally preferred now, no? If so let's switch to the generally accepted best practice, although I know there are active debates on that! https://twitter.com/search?q=%22don%27t%20use%22%20%20python%20&src=typed_query&f=top

Yep, I'm generally in favour of an environment.yml installed with conda over a requirement.txt. I would say, however, that if we go down that route then maybe it's worth explicitly specifying that the TDS preference is to use anaconda python over base python?

Also worth noting that we'd want to make sure that the environment was platform-independent so would probable be best to use the --from-history flag when exporting the environment.

@Robinlovelace
Copy link
Member

Yep, I'm generally in favour of an environment.yml installed with conda over a requirement.txt. I would say, however, that if we go down that route then maybe it's worth explicitly specifying that the TDS preference is to use anaconda python over base python?

Sounds like a good plan. Most Python developers I know use miniconda not Anaconda but I understand the latter is better for beginners so happy with that.

@ksuchak1990
Copy link
Author

Sounds like a good plan. Most Python developers I know use miniconda not Anaconda but I understand the latter is better for beginners so happy with that.

Yep, I also prefer miniconda, but I think most of the python taught materials I've interacted with have used anaconda so as to reduce the time spent installing extra packages after installation.

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

No branches or pull requests

3 participants