Suggestions for code organization#16
Closed
ian-ross wants to merge 1 commit into
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I noticed some things that I would normally set up differently in a Python project. This PR shows some suggestions:
datadirectory, and also to use an environment variable to provide a default location for input files (AEIC_DATA_DIRor something). Then you can use a little wrapper around anywhere that you open files to look first in the current working directory, and then in the given data directory. This allows you to easily have job-specific data files and fall back onto defaults. I've moved things around in this PR and I've added a utility function to do the file lookup. See the example workflow below to see how it works.srcdirectory (which is a good idea), you need to do it properly. If you find yourself writingimport src..., something is not right. The idea of using asrcdirectory is that it forces you to install your code in the same way as the users of the package you're writing will have to. That means that you should install your package as an editable install usingpip. (See the workflow suggestion and the references below.)dependenciessection of thepyproject.tomlfile is for dependencies you need when using your code. Dependencies needed for development activities like building documentation go into a separatedevdependencies group. And you should list the actual dependencies in yourpyproject.toml(in this case, justnumpyandscipy, but it's a good habit to get into).from something import *. It causes all sorts of problems in the end, for a few seconds of convenience at the start.BADApackage, for example, there are lots of cases whereOptional[float]values are multiplied by aFloatOrNDArrayvalue. The linting tool that I use (eitherpyrightorruff, not sure which one this message comes from) reports these as Operation "*" not supported for types "FloatOrNDArray" and "float | None". It also catches things like using dictionary access to access attributes of dataclass values, for example usingself.aircraft_parameters['c_tc4']instead ofself.aircraft_parameters.c_tc4. Using fussy annoying tools saves you time in the end: if you get rid of all the irritating red squiggles these tools show you before you try running things, you will avoid a lot of potential errors. (I've more or less settled on usingpyrightandruff, annoying as they are.)Example workflow
References
srclayout: https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/