From 1ca0d67a8d4788ff77c1a65b512dcb5d1491d081 Mon Sep 17 00:00:00 2001 From: GP Saggese Date: Sat, 12 Apr 2025 10:06:20 -0400 Subject: [PATCH] Improve --- config_root/config/config_list_builder.py | 14 +-- config_root/config/config_utils.py | 6 +- .../config/test/test_config_builder.py | 4 +- config_root/config/test/test_config_utils.py | 2 +- helpers/henv.py | 1 - helpers/hio.py | 2 +- helpers/hlatex.py | 3 - helpers/hobject.py | 4 +- helpers/hpandas.py | 10 +- helpers/lib_tasks_docker.py | 8 +- helpers/test/test_hcache.py | 3 - helpers/test/test_hdbg.py | 4 +- linters/test/test_amp_dev_scripts.py | 111 ------------------ linters/test/test_amp_fix_md_links.py | 8 +- 14 files changed, 31 insertions(+), 149 deletions(-) diff --git a/config_root/config/config_list_builder.py b/config_root/config/config_list_builder.py index 592e102ac..f6b9c6dcd 100644 --- a/config_root/config/config_list_builder.py +++ b/config_root/config/config_list_builder.py @@ -63,7 +63,7 @@ def get_universe_top_n(universe: List[Any], n: Optional[int]) -> List[Any]: # ############################################################################# -# TODO(gp): -> get_time_interval +# TODO(gp): GFI -> get_time_interval def get_period(period: str) -> Tuple[pd.Timestamp, pd.Timestamp]: """ Get start and end timestamps from the specified period. @@ -193,11 +193,11 @@ def build_config_list_varying_asset_id( # ############################################################################# -# TODO(gp): -> ...varying_asset_tiles +# TODO(gp): GFI -> ...varying_asset_tiles def build_config_list_varying_universe_tiles( config_list: cconfig.ConfigList, universe_tile_id: cconfig.CompoundKey, - # TODO(gp): -> asset_tiles + # TODO(gp): GFI -> asset_tiles universe_tiles: List[List[int]], ) -> cconfig.ConfigList: """ @@ -226,7 +226,7 @@ def build_config_list_varying_universe_tiles( return config_list_out -# TODO(gp): -> ...varying_period_tiles +# TODO(gp): GFI -> ...varying_period_tiles def build_config_list_varying_tiled_periods( config_list: cconfig.ConfigList, start_timestamp: pd.Timestamp, @@ -255,7 +255,7 @@ def build_config_list_varying_tiled_periods( hdateti.dassert_has_tz(start_timestamp) hdateti.dassert_has_tz(end_timestamp) hdbg.dassert_lte(start_timestamp, end_timestamp) - # TODO(gp): Check that the lookback is > 0. + # TODO(gp): GFI Check that the lookback is > 0. lookback = pd.Timedelta(lookback_as_pd_str) # configs = [] @@ -311,7 +311,7 @@ def build_config_list_varying_tiled_periods( # ############################################################################# -# TODO(gp): -> build_config_list_using_equal_asset_tiles +# TODO(gp): GFI -> build_config_list_using_equal_asset_tiles def build_config_list_with_tiled_universe( config_list: cconfig.ConfigList, asset_ids: List[int] ) -> cconfig.ConfigList: @@ -367,7 +367,7 @@ def apply_build_config_list( return config_list_out -# TODO(gp): -> build_config_list_using_equal_asset_and_period_tiles +# TODO(gp): GFI -> build_config_list_using_equal_asset_and_period_tiles def build_config_list_with_tiled_universe_and_periods( config_list: cconfig.ConfigList, ) -> cconfig.ConfigList: diff --git a/config_root/config/config_utils.py b/config_root/config/config_utils.py index e80753736..3fc2ada19 100644 --- a/config_root/config/config_utils.py +++ b/config_root/config/config_utils.py @@ -86,7 +86,7 @@ def replace_shared_dir_paths( return new_config -# TODO(gp): Add unit tests. +# TODO(gp): GSI: Add unit tests. def sort_config_string(txt: str) -> str: """ Sort a string representing a Config in alphabetical order by the first @@ -475,7 +475,7 @@ def diff_configs(configs: Iterable[crococon.Config]) -> List[crococon.Config]: # ############################################################################# -# TODO(gp): Is this private? +# TODO(gp): GFI: Is this private? def convert_to_series(config: crococon.Config) -> pd.Series: """ Convert a config into a flattened series representation. @@ -501,7 +501,7 @@ def convert_to_series(config: crococon.Config) -> pd.Series: return srs -# TODO(gp): Is this private? +# TODO(gp): GFI: Is this private? def convert_to_dataframe(configs: Iterable[crococon.Config]) -> pd.DataFrame: """ Convert multiple configs into flattened dataframe representation. diff --git a/config_root/config/test/test_config_builder.py b/config_root/config/test/test_config_builder.py index 493bd9c56..ce484ab98 100644 --- a/config_root/config/test/test_config_builder.py +++ b/config_root/config/test/test_config_builder.py @@ -32,7 +32,7 @@ def _build_test_config_list( # ############################################################################# -# TODO(gp): -> Test_get_config_list_from_builder1 +# TODO(gp): GFI -> Test_get_config_list_from_builder1 class TestGetConfigsFromBuilder1(hunitest.TestCase): def test1(self) -> None: """ @@ -49,7 +49,7 @@ def test1(self) -> None: # ############################################################################# -# TODO(gp): -> Test_get_config_from_env1 +# TODO(gp): GFI -> Test_get_config_from_env1 class TestGetConfigFromEnv(hunitest.TestCase): def test_no_env_variables(self) -> None: """ diff --git a/config_root/config/test/test_config_utils.py b/config_root/config/test/test_config_utils.py index 7e41bedce..e7b903278 100644 --- a/config_root/config/test/test_config_utils.py +++ b/config_root/config/test/test_config_utils.py @@ -77,7 +77,7 @@ def _get_test_config4() -> cconfig.Config: # ############################################################################# -# TODO(gp): -> validate_config_list +# TODO(gp): GFI -> validate_config_list? class Test_validate_configs1(hunitest.TestCase): def test_check_same_configs_error(self) -> None: diff --git a/helpers/henv.py b/helpers/henv.py index 25af1ea1a..53769b154 100644 --- a/helpers/henv.py +++ b/helpers/henv.py @@ -283,7 +283,6 @@ def _git_log(num_commits: int = 5, my_commits: bool = False) -> str: if my_commits: # This doesn't work in a container if the user relies on `~/.gitconfig` to # set the user name. - # TODO(gp): We should use `get_git_name()`. cmd.append("--author $(git config user.name)") cmd = " ".join(cmd) data: Tuple[int, str] = hsystem.system_to_string(cmd) diff --git a/helpers/hio.py b/helpers/hio.py index 18c1f8624..5bbdd07b8 100644 --- a/helpers/hio.py +++ b/helpers/hio.py @@ -79,7 +79,7 @@ def listdir( cmd.append(r'-not -path "*/\.git/*"') cmd = " ".join(cmd) _, output = hsystem.system_to_string(cmd) - # TODO(gp): -> system_to_files + # TODO(gp): GSI -> system_to_files paths = [path for path in output.split("\n") if path != ""] _LOG.debug("Found %s paths in %s", len(paths), dir_name) _LOG.debug("\n".join(paths)) diff --git a/helpers/hlatex.py b/helpers/hlatex.py index 98da2cb75..ef5ae6e40 100644 --- a/helpers/hlatex.py +++ b/helpers/hlatex.py @@ -5,9 +5,6 @@ import helpers.hio as hio import helpers.hprint as hprint -# TODO(gp): Consider using `pypandoc` instead of calling `pandoc` directly. -# https://boisgera.github.io/pandoc - # TODO(gp): Add a switch to keep the tmp files or delete them. def convert_pandoc_md_to_latex(txt: str) -> str: diff --git a/helpers/hobject.py b/helpers/hobject.py index 1534dc60b..fef6f2187 100644 --- a/helpers/hobject.py +++ b/helpers/hobject.py @@ -472,8 +472,8 @@ def to_config_str(self) -> str: # ############################################################################# -# TODO(gp): CleanUp. This is for testing and should be in hobject_test.py. -# TODO(gp): -> check_object_signature +# TODO(gp): GSI. This is for testing and should be in hobject_test.py. +# TODO(gp): GSI -> check_object_signature def test_object_signature( self_: Any, obj: Any, *, remove_lines_regex: Optional[str] = None ) -> None: diff --git a/helpers/hpandas.py b/helpers/hpandas.py index 8a05d0d90..787e341a8 100644 --- a/helpers/hpandas.py +++ b/helpers/hpandas.py @@ -141,7 +141,7 @@ def dassert_unique_index( hdbg.dassert(index.is_unique, msg=msg, *args) -# TODO(gp): @all Add unit tests. +# TODO(gp): GSI. Add unit tests. def dassert_increasing_index( obj: Union[pd.Index, pd.DataFrame, pd.Series], msg: Optional[str] = None, @@ -181,7 +181,7 @@ def dassert_increasing_index( hdbg.dassert(index.is_monotonic_increasing, msg=msg, *args) -# TODO(gp): @all Add more info in case of failures and unit tests. +# TODO(gp): GSI. Add more info in case of failures and unit tests. def dassert_strictly_increasing_index( obj: Union[pd.Index, pd.DataFrame, pd.Series], msg: Optional[str] = None, @@ -210,7 +210,7 @@ def dassert_monotonic_index( hdbg.dassert(cond, msg=msg, *args) -# TODO(Paul): @gp -> dassert_datetime_indexed_df +# TODO(Paul): GSI -> dassert_datetime_indexed_df def dassert_time_indexed_df( df: pd.DataFrame, allow_empty: bool, strictly_increasing: bool ) -> None: @@ -1776,11 +1776,11 @@ def get_random_df( # ############################################################################# -# TODO(gp): -> AxisNameSet +# TODO(gp): GSI -> AxisNameSet ColumnSet = Optional[Union[str, List[str]]] -# TODO(gp): -> _resolve_axis_names +# TODO(gp): GSI -> _resolve_axis_names def _resolve_column_names( column_set: ColumnSet, columns: Union[List[str], pd.Index], diff --git a/helpers/lib_tasks_docker.py b/helpers/lib_tasks_docker.py index a3355fa63..555127de5 100644 --- a/helpers/lib_tasks_docker.py +++ b/helpers/lib_tasks_docker.py @@ -675,7 +675,7 @@ def _generate_docker_compose_file( ], "extends": "app", "network_mode": "${NETWORK_MODE:-bridge}", - # TODO(gp): Rename `AM_PORT`. + # TODO(gp): Rename `CSFY_PORT`. "ports": [ "${PORT}:${PORT}", ], @@ -981,7 +981,7 @@ def _get_base_image(base_image: str) -> str: :return: e.g., *****.dkr.ecr.us-east-1.amazonaws.com/amp """ if base_image == "": - # TODO(gp): Use os.path.join. + # TODO(gp): GFI. Use os.path.join. base_image = ( hlitauti.get_default_param("CSFY_ECR_BASE_PATH") + "/" @@ -1008,8 +1008,8 @@ def _get_base_image(base_image: str) -> str: # return tag_name -# TODO(gp): Consider using a token "latest" in version, so that it's always a -# string and we avoid a special behavior encoded in None. +# TODO(gp): GSI: Consider using a token "latest" in version, so that it's +# always a string and we avoid a special behavior encoded in None. def get_image( base_image: str, stage: str, diff --git a/helpers/test/test_hcache.py b/helpers/test/test_hcache.py index d8a5eaa38..a6fbbd5fd 100644 --- a/helpers/test/test_hcache.py +++ b/helpers/test/test_hcache.py @@ -15,9 +15,6 @@ _LOG = logging.getLogger(__name__) -# TODO(gp): Do not commit this. -# _LOG.debug = _LOG.info - # TODO(gp): Use hprint.log_frame def _LOG_frame(txt: str) -> None: diff --git a/helpers/test/test_hdbg.py b/helpers/test/test_hdbg.py index 25bc2666c..de99165c1 100644 --- a/helpers/test/test_hdbg.py +++ b/helpers/test/test_hdbg.py @@ -12,8 +12,8 @@ # ############################################################################# -# TODO(gp): Use a self.assert_equal() instead of a check_string() since this -# code needs to be stable. +# TODO(gp): GSI: Use a self.assert_equal() instead of a check_string() since +# this code needs to be stable. class Test_dassert1(hunitest.TestCase): """ Test `dassert()`. diff --git a/linters/test/test_amp_dev_scripts.py b/linters/test/test_amp_dev_scripts.py index 545b9bac2..9a9f1aa49 100644 --- a/linters/test/test_amp_dev_scripts.py +++ b/linters/test/test_amp_dev_scripts.py @@ -396,114 +396,3 @@ def test1(self) -> None: # Comment before initializing. class TestAnother(): pass - -if __name__ == "main": - txt = "hello" - m = re.search("\s", txt) - n = nltk.word_tokenize(txt) - hdbg.dassert_path_exists("filename.txt") - ''' - return txt - - @staticmethod - def _get_ipynb_contents1() -> str: - contents_ipynb = r""" -{ - "cells": [ - { - "cell_type": "markdown", - "metadata": {}, - "source": [ - "# Imports" - ] - }, - { - "cell_type": "code", - "execution_count": null, - "metadata": {}, - "outputs": [], - "source": [ - "import pandas as pd\n", - "import re\n", - "\n", - "# TODO: Fix.\n", - "res = re.findall(r\"[a-z]+\", \"some text\")\n", - "\n", - "\n" - ] - } - ], - "metadata": { - "kernelspec": { - "display_name": "Python 3 (ipykernel)", - "language": "python", - "name": "python3" - } - }, - "nbformat": 4, - "nbformat_minor": 5 -} - """ - return contents_ipynb - - def _run_linter( - self, - file_name: str, - linter_log: str, - as_system_call: bool, - ) -> str: - if as_system_call: - cmd = [] - cmd.append(f"linters/base.py --files {file_name}") - cmd_as_str = " ".join(cmd) - ## We need to ignore the errors reported by the script, since it - ## represents how many lints were found. - suppress_output = _LOG.getEffectiveLevel() > logging.DEBUG - hsystem.system( - cmd_as_str, - abort_on_error=False, - suppress_output=suppress_output, - output_file=linter_log, - ) - cmd_rm = f"git restore --staged {file_name}" - hsystem.system(cmd_rm, abort_on_error=False) - else: - logger_verbosity = hdbg.get_logger_verbosity() - parser = libase._parse() - args = parser.parse_args( - [ - "-f", - file_name, - "--linter_log", - linter_log, - # TODO(gp): Avoid to call the logger. - "-v", - "ERROR", - ] - ) - libase._main(args) - hdbg.init_logger(logger_verbosity) - - # Read log. - _LOG.debug("linter_log=%s", linter_log) - txt = hio.from_file(linter_log) - # Process log. - output = [] - output.append("# linter log") - for line in txt.split("\n"): - # Remove the line: - # Cmd line='.../linter.py -f input.py --linter_log ./linter.log' - if "cmd line=" in line: - continue - # Filter out code rate because of #2241. - if "Your code has been rated at" in line: - continue - output.append(line) - # Read output. - _LOG.debug("file_name=%s", file_name) - output.append("# linter file") - txt = hio.from_file(file_name) - output.extend(txt.split("\n")) - # ////////////// - output_as_str = "\n".join(output) - return output_as_str diff --git a/linters/test/test_amp_fix_md_links.py b/linters/test/test_amp_fix_md_links.py index 779584369..241dc428e 100644 --- a/linters/test/test_amp_fix_md_links.py +++ b/linters/test/test_amp_fix_md_links.py @@ -55,8 +55,8 @@ def write_input_file(self, txt: str, file_name: str) -> str: hio.to_file(file_path, txt) return file_path - # TODO(gp): To outsource. Break into smaller tests. If one of these fails, - # it's hard to debug. + # TODO(gp): GSI. Break into smaller tests. If one of these fails, it's hard + # to debug. def test1(self) -> None: """ Test fixing link formatting in a Markdown file. @@ -205,8 +205,8 @@ def test2(self) -> None: output = _get_output_string(out_warnings, updated_lines) self.check_string(output, purify_text=True) - # TODO(gp): To outsource. Break into smaller tests. If one of these fails, - # it's hard to debug. + # TODO(gp): GSI. Break into smaller tests. If one of these fails, it's hard + # to debug. def test3(self) -> None: """ Test the mix of Markdown and HTML-style links.