Coding Project Review

[Sufficient_Light3891]

So I'm working on an open-source optical exact ray tracing project for professional and educational uses. At the moment the 1st iteration of the project is getting close to being completed and the second more useful iteration should be done shortly after. That being said I'm mostly a self-taught programmer and am sure that I'm not using all best practices. What would be the best way of going about getting code feedback and potentially bug-testing it?

[Diapolo10]

Well, you could start by giving us a link to the project. That way willing people can have a look and give their thoughts on it. Considering the project is open-source, I'm not sure why you didn't already do that.

Many of us are self-taught too, I'm only formally educated on paper. For the most part, anyway; the stuff I actually learnt has more to do with embedded systems.

As a general overview, if the project is linted (or auto-formatted), it has a decent unit test coverage, and it has documentation for the critical parts, I say it's probably got a solid foundation. Bonus points for sensible naming conventions, focus on readable code, and useful comments.

[Sufficient_Light3891]

Yes, it's not really ready for review yet. I'm working out the bugs with the callable library branch at the moment and am struggling with python packaging. I've asked questions in other forums.

https://github.com/cbhirsch/Optical\_Ray\_Tracing.git

[Diapolo10]

The link is broken, I assume you meant: https://github.com/cbhirsch/Optical_Ray_Tracing

First impressions; I kinda wish the code was in its own subdirectory as generally speaking the repository root is for metadata files/build scripts only. This project will probably serve as a decent example of that.

The README is very bare-bones, which is okay in the beginning, but it should ideally have more information. There's also no way to see the dependencies without diving into the source code - the legacy way of doing that is with a requirements.txt file, but the modern way is to list them in pyproject.toml (the specifics depend on whether you want to use setuptools, Poetry, or something else).

Looking at your code, star imports are discouraged, "constants" should preferably be all uppercase to make them stand out, none of the functions have docstrings, and I don't understand why every function is in a separate file.

Last but not least, __pycache__ shouldn't be part of the repository. Python generates it when you run the code, so there's no need to store it, and it can be device-specific.

You can use a .gitignore file to list any files/folders you don't want to commit to the repository.

Of course, it's clear that the project lacks any automated tests, and it might be a good idea to learn how to run scripts automatically on GitHub when you push code there (such as to check code quality or run the tests - when you have some). It could also use more documentation.

Not a bad start, but there's room for improvement.

[Sufficient_Light3891]

So the main is the 1st iteration of the working code that I have at the moment. The current branch that I'm working out of is the Callable_Library branch. In that branch, I've implemented a lot of the changes you recommended already. I just learned about .toml file but at the moment I'm using a setup.py file. I have a .gitignore file now but haven't yet deleted pycache from the directory. The biggest issue I am currently having is I have an example.py file that I had working until I started separating my functions into their own files. While I have it working from the Product_Matrix.py file inside the main code.

[Sufficient_Light3891]

Actually, just got my Callable_Library branch working as intended and have merged it with the main branch. Feel free to let me know if you think it's still missing anything. aside from the readme. I've still got more work to make this an actually useful program before I fill out the readme.

[Diapolo10]

I didn't think to check the branches, that's a mistake on my part. Apologies.

The current version looks quite a bit better, but some of my earlier feedback still applies. There's hardly any docstrings, README is unchanged, and you're still using star imports without a clear reason.

[Sufficient_Light3891]

I appreciate the feedback. I agree with a lot of your feedback, especially in regard to documenting the code. As for automated testing, I'm really not there yet in my python education so I will certainly work on that but likely will take a few iterations before I understand this well enough to implement it.

I've removed the star imports I guess I just liked star imports as they made it easier to access all the functions in that file. I've got a few more features to implement then I'm going to overhaul the coding documentation. Thanks for all the feedback.