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

Use "TODO" comments to track deprecated codes or workarounds for old versions #3705

Closed
4 tasks done
seisman opened this issue Dec 21, 2024 · 0 comments
Closed
4 tasks done
Labels
maintenance Boring but important stuff for the core devs
Milestone

Comments

@seisman
Copy link
Member

seisman commented Dec 21, 2024

Sometimes, we may need to mark part of the codes for future reference. For example,

  1. Deprecated parameters/syntax/functions/methods should be removed in the future. E.g.,

    pygmt/pygmt/clib/session.py

    Lines 1940 to 1944 in 5702209

    msg = (
    "API function 'Session.virtualfile_from_data()' has been deprecated since "
    "v0.13.0 and will be removed in v0.15.0. Use 'Session.virtualfile_in()' "
    "instead."
    )
  2. Workarounds for old package versions should be removed after we drop the support of old versions. E.g.,
    # Workarounds for pandas < 2.2. Following SPEC 0, pandas 2.1 should be dropped in
    # 2025 Q3, so it's likely we can remove the workaround in PyGMT v0.17.0.
    if (
    Version(pd.__version__) < Version("2.2") # pandas < 2.2 only.
    and hasattr(data, "dtype") # NumPy array or pandas objects only.
    and hasattr(data.dtype, "numpy_dtype") # pandas dtypes only.
    and data.dtype.kind in "iuf" # Numeric dtypes only.
    ): # pandas Series/Index with pandas nullable numeric dtypes.

    if Version(__gmt_version__) < Version("6.5.0"):

    # Keep this until we require numpy to be >=2.0
    # Address https://github.com/GenericMappingTools/pygmt/issues/2628.
  3. More other cases.

Currently, for case 1, we use the grep --include="*.py" -r vX.Y.Z command to find the matches, but for case 2, we don't have a way to track them.

To better track codes that should be removed/refactored in the future, I suggest using the TODO comments in our source codes. It should be noted that TODO comments should not be abused for marking unimplemented features or incomplete documentation, which should be tracked by issues instead.

There is no style guide like PEP8 for TODO comments. Looking at this todo tool (https://github.com/ianlewis/todos), the ruff's TODO linter, and also searching for the web, here are the TODO comment styles I've found:

# TODO: Description 
# TODO(somebody): Description
# TODO@somebody: Description
# TODO(#1234567): Description

Since we're using TODO comments for tracking deprecated codes/workarounds, I think we can use the following style:

# TODO(Package>=vX.Y.Z): Description

e.g.,

# TODO(GMT>=6.5.0): Remove the workaround below.
# TOOD(pandas>=2.0): Remove the workaround for pandas<2.0.
# TODO(PyGMT>=0.16.0): Remove the deprecated parameter/function/method ...

Then we can use grep "# TODO" **/* to find all TODOs. Some editors even have plugins to highlight TODO comments or show all TODOs in a compact way.

If agreed, we should do:

@seisman seisman added the discussions Need more discussion before taking further actions label Dec 21, 2024
@seisman seisman changed the title Use "TODO" comments to mark future changes Use "TODO" comments to track deprecated codes or workarounds for old versions Dec 23, 2024
@seisman seisman added maintenance Boring but important stuff for the core devs and removed discussions Need more discussion before taking further actions labels Dec 26, 2024
@seisman seisman added this to the 0.14.0 milestone Dec 26, 2024
@seisman seisman closed this as completed Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Development

No branches or pull requests

1 participant