Packaging Update - Githubissues

I've made some changes to the project so that each of the scripts no longer depend on hard-coded file paths, and it's possible to import the files without running everything. Each script has been bundled into a function that takes input/output paths as arguments, and these can be set from the command line. I've also set up the project as an installable Python package, so you can do things like:

from SciViPy import bout_cartesian_convert

bout_cartesian_convert(args...)

Instructions for installing everything are in the updated README. If you still wish to run the individual files as scripts, you can still do so, as I've implemented a command line interface for each of them (you can run python some_script.py --help to get a nice explanation of how to run each one).

Unfortunately, converting a script to a module containing a function means indenting an entire file, so according to this PR I've rewritten almost everything. In reality, not much has changed, and I've tried to avoid changing any of the core logic where possible.

I've submitted this as a work-in-progress, as I don't yet have access to sufficient testing data (Issue https://github.com/DomLonghorn/SciViPy/issues/37), so the closest I can get to ensuring that everything actually works is running flake8 to make sure I haven't made any typos or syntax errors. I was sent a Jorek .vtk file, but it looks like I'll need a CAD file too (not sure what that is in this context?), and I'll need BOUT files,.xyz files, .txt files for time_reader, .csv files for crystal_vis, and a few images for gif_maker. Looking at the comments throughout the code, it looks like it's usually run on large datasets and it can take up to an hour to finish. For testing purposes, I'd only need 1 or 2 files of each type, and they don't need to contain anything interesting -- just enough so I can verify that the code is working as expected.

Changes:

Added setup.py, setup.cfg, pyproject.toml, __init__.py. These allow the package to be pip installable. There's quite a lot of confusing info out there on Python packaging, but I usually use this as a reference for setting it up using setuptools with a setup.cfg file. This will likely be deprecated in a few months, as they're moving over to a method that puts everything in pyproject.toml instead.
Renamed Scripts folder to the project name SciViPy. It's a common way to structure a Python project, and it makes packaging a bit easier.
Updated all scripts to use the idiom if __name__ == "__main__", so they can run as scripts or be imported as Python modules.
All scripts using argparse to manage command line interface. No more hard-coded filepaths.
Got the docs working.
Some minor code alterations, e.g:
- Use pathlib.Path over os path functions.
- Avoid lists as default function params. Mutable containers as default arguments can cause weird bugs.
- Not returning from print statements
Updated README
Amended docstrings throughout, using NumPy style

Planned further updates:

Testing overhaul: each test script should call the main functions with known inputs, and compare against known answers. This should be done automatically (i.e. no human checking required), and should use sufficiently small datasets that the tests can run in seconds rather than minutes. I can get this set up, but I'll need access to a few files of each type, including weirder things like BOUT grid files and the CAD file that Jorek visualisation needs.
Continuous Integration: once testing is automated, it's relatively easy to get GitHub to run tests each time somebody submits a PR or pushes their changes. We could also automatically run black on submitted code and run flake8 checks.
Style overhaul: I'd recommend having a look over PEP8, particularly on naming conventions. When I'm reading Python code, I tend to assume that CapsNames are classes, while snake_case_names are functions or variables. However, I realise that it won't be possible to keep this consistent throughout the code, as paraview doesn't appear to be written with any Python style in mind.
Use pandas for anything .csv related -- it's just really good at managing columns of data.
Rather than have functions operate on entire directories, have them operate on individual files. It'll keep the functions internals much cleaner, avoiding a whole layer of for looping, and the loop over a directory can be performed outside the main function call.

Hey, sorry for the long pause on this PR, and thanks for adding in some test data. I've made some further updates:

Code Refactoring

Gif_Maker:
- renamed make_gif
- Simplifies getting filenames using pathlib.Path.rglob to get files).
- Now returns the path to the file it created.
time_reader:
- Uses pandas.read_fwf (fixed-width-file) to read input data file.
- Replaced loops with some NumPy magic.
- Now returns a pandas.DataFrame with the new timesteps and indices. Does not write a file (though the user can do so easily using result.to_csv("myfile.csv")).
- TODO: I think I may have found a bug in the code, but it's difficult to tell since I'm not sure how the function should be used in practice. It looks like the function is calculating evenly spaced time steps between the second index in the file and the $(N-1)$-th index, but I'm not sure whether that's by design. I've left a fix in the code as a comment in case the intent was to go from the 1st to the $N$-th index.

Packaging

Upgraded to the new way of doing things, where everything is specified in the file pyproject.toml. This is recommended by PEP 517/518/621.
You may need to run pip install --upgrade pip to get it to work, as some features were only recently added to the standard build tools.

Testing

Added tests folder, containing tests that can be run using pytest. The README explains how to run them. I'd highly advise reading up on pytest when you have a few minutes. The tests I've added already should give you some idea of how things work.
Sorry for the duplication with Testing! Pytest discovers tests based on the names of files/functions/classes, so some of the scripts in here would interfere. I opted to just add a new directory, and the old tests should be migrated here as each is upgraded to be pytest compatible.
I've added tests for make_gif and time_reader. The reason I was able to work on these functions in the first place was because I had test data available, meaning I could ensure that any changes I made weren't breaking anything.
Ideally, it'd be great to see some testing data for the other scripts/functions.

Automation

Upgraded the black workflow to use the official action. It'll run whenever any Python files are updated, and will run in other branches besides main.
Added a unit testing workflow that runs pytest on the tests folder.
The tests won't run in this PR, but you can see the results in the PlasmaFAIR fork: black and tests.

I think that's about as far as I can take this PR without more testing data. I think that getting more tests in is the most important next step, as I'm currently not able to check whether any of the Paraview stuff works.

Let me know if you'd like me to revert any changes to the code (I realise I may have been a little overzealous in places). I'm also happy to explain any changes if you're not sure how they work.

DomLonghorn / SciViPy