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

Python code cleanup #52473

Open
7 tasks
damien opened this issue Oct 26, 2021 · 0 comments
Open
7 tasks

Python code cleanup #52473

damien opened this issue Oct 26, 2021 · 0 comments
Assignees
Labels
[Python] Code made in Python

Comments

@damien
Copy link
Contributor

damien commented Oct 26, 2021

Is your feature request related to a problem? Please describe.

There's a fair amount of Python code in this project:

  • The utils/ directory, which contains:
    • a script for generating isometric views of CDDA tilesets
    • a tool named "building utility" which seems to be used to combine multi-cell map tiles
  • The tools directory, which contains a mix of configuration files, editor plugins, and plethora of python code

Of the Python code in this repository, a some Python present in the tools directory could benefit from a some (I hope) straightforward quality improvements or cleanup:

  • There are a fair few scripts under tools/ which are lacking easily discoverable context or documentation describing what they're used for or how they should be used
  • It's unclear to me which Python scripts are still relevant today. Some may be in need of updating or be better of being removed if they are no longer useful.
  • Some Python scripts in the repository depend on Python libraries that are not present in Python's standard libraries, making them unusable without first debugging errors to discover what dependencies are missing. A couple instances of this I found include:
  • Currently, no testing is done of any of this code. I believe this is worth implementing for development tooling we want functional over long periods of time and through changes in the development or CI environment (like the move from Jenkins to Github Actions).

Describe the solution you'd like

I think there are a few tracks of work that can be done here to bring the quality and usability of existing Python code up to the quality standards of the rest of this repository:

  1. Audit currently existing Python code and categorize them into one of three buckets:
    • Keep: Code we care about and wish to continue to maintain
    • Revise: Code that was useful, but is not currently usable for one reason or another (I believe the venerable changelog generator falls into this category)
    • Remove: Code that we no longer wish to maintain. This could include code that is no longer useful or that has become irrelevant due to other changes in the broader project or development ecosystem.
  2. Delete code that fell into the Remove bucket. This lowers the total amount of Python code that needs to be maintained and is that much less code laying around to distract from things that are relevant to the CDDA development.
  3. Decide on a canonical Python version. We seem to be defaulting to whatever python3 resolves to in Ubuntu and on Docker's ubuntu-latest, which at this point in time is Python 3.8.10 on Ubuntu 20.04LTS aka focal. On the latest stable Ubuntu release this resolves to Python 3.9.7 for Ubuntu 21.10 aka impish. This is important because:
    • Python stabilized support for the typing stdlib module in 3.9, which some scripts in tools/ use
    • The cadence of Python's language releases does not match those of Ubuntu, meaning if we want Python versions across development environments to match what we run on CI/CD, we'll need to pin things to either whatever the CI/CD Ubuntu via Docker APT package resolves to (currently Docker's ubuntu-latest resolves to Ubuntu 20.04 (focal) which pulls Python 3.8.10) or to a fixed version of Python we update as needed.
    • Non Ubuntu development environments would benefit from having a way to determine the appropriate version of Python, which is currently implicit and undocumented
  4. Configure a dependency manager (e.g. pip, Poetry, whatever) to declare non-stdlib Python dependencies and their versions from pipy (the canonical Python package registry)
    • This should resolve existing issues about undocumented or uninstalled Python dependencies
    • Such dependency managers also provide guarantees that developers running Python code are using the same versions of currently implicit dependencies
    • This opens up avenues to pull in more 3rd party Python code as needed if there is interest to do so (json parsers, linters, test frameworks for Python, etc)
  5. Update scripts that fall into the Keep bucket:
    • Ensure all scripts can be run through the chosen dependency manager and Python version
    • Stub out some simple tests to verify simple behavior (at a minimum, verify known outputs vs known inputs)
  6. Update scripts that fall into the Revise bucket using the same process as those in the Keep bucket. Additionally:
    • Revisit/Reevaluate original purpose of said code
    • Update/Revise these scripts such that they're useful and make sense in the current context of the CDDA repository

Describe alternatives you've considered

  • Leaving things as they are
  • Targeted changes/edits/revisions
    This is a valid approach for specific cases where we know we want to keep specific Python code, but:
    • I suspect this would result in ongoing inconsistencies within Python code in this repository.
    • This doesn't address code clean up for Python code that is no longer relevant.
    • We'd likely end up duplicating some work we could avoid with a more holistic approach (e.g. dependencies, code style, documentation)
    • This doesn't address the underlying problem of not having a consistent approach to managing Python code in this repository

Additional context

  • Pyenv, a tool for managing Python versions
  • Poetry, a dependency manager for Python
  • @I-am-Erk 's CDDA-Tiles project makes use of Python code in the tools/ directory outside of the context of this project. We could probably figure out better ways of packaging tools like this for easier use by pushing some of CDDA's Python dev tools to Pypi once we've cleaned things up a bit.

Progress

  • Python code cleanup, PoC: Changelog #52477
  • Audit Python code and determine what we Keep, Revise, or Remove
  • Formalize a Python version
  • Select and formalize a Python dependency manager
  • Update scripts marked as Keep to use selected Python version and dependency manager
  • Update scripts marked as Revise to use selected Python version, dependency manager, and refactor/redesign as needed
  • Delete scripts marked as Remove ; more work may need to be done here to figure out what is and is not actively in use
@damien damien self-assigned this Oct 26, 2021
@damien damien added the [Python] Code made in Python label Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Python] Code made in Python
Projects
None yet
Development

No branches or pull requests

1 participant