Skip to content

Replace deprecated pkg_resources with importlib.resources and improve path handling#76

Merged
ilaflott merged 6 commits intomainfrom
copilot/fix-75
Sep 23, 2025
Merged

Replace deprecated pkg_resources with importlib.resources and improve path handling#76
ilaflott merged 6 commits intomainfrom
copilot/fix-75

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Sep 18, 2025

Replace deprecated pkg_resources with importlib.resources and improve path handling.

Analysis and Plan

  • Identify all usages of pkg_resources in the codebase
  • Confirm schema files are available via git submodule
  • Test current functionality and ensure all tests pass
  • Verify importlib.resources provides equivalent functionality
  • Replace pkg_resources import with importlib.resources in 3 files:
    • fms_yaml_tools/field_table/is_valid_field_table_yaml.py
    • fms_yaml_tools/data_table/is_valid_data_table_yaml.py
    • fms_yaml_tools/diag_table/is_valid_diag_table_yaml.py
  • Test all changes work correctly
  • Run full test suite to ensure no regressions
  • Verify deprecation warnings are eliminated
  • Update CI to remove Python 3.8 testing and add Python 3.12
  • Use pathlib.Path idiomatically with / operator
  • Restore debug verbosity functionality
  • Fix flake8 linting issues in docstring formatting

Changes Made

Updated 3 validation files to:

  • Replace import pkg_resourcesimport importlib.resources
  • Use importlib.resources.files('fms_yaml_tools') / "schema/..." instead of string concatenation
  • Pass pathlib.Path objects to validate_yaml instead of strings

Updated validate_schema.py to:

  • Restore verbose debug printing functionality with verboseprint lambda function
  • Keep debug parameter fully functional for backward compatibility
  • Handle both string and Path objects for file paths using str() conversion
  • Fixed flake8 whitespace issues in docstring

Updated CI configuration:

  • Removed Python 3.8 from test matrix (end of life)
  • Added Python 3.12 for broader coverage

Verification

✅ All 42 tests pass
✅ CLI tools function correctly with pathlib.Path objects
✅ No deprecation warnings when importing with -W error::DeprecationWarning
✅ Schema files remain accessible
✅ Debug parameter fully functional with verbose output when enabled
✅ Idiomatic pathlib.Path usage with / operator
✅ CI updated to test modern Python versions (3.9-3.12)
✅ flake8 linting passes with no warnings

The deprecation warnings shown in the issue will no longer appear when FRE users run frerun with fms-yaml-tools. The code now uses modern, idiomatic Python patterns while maintaining full debug functionality.

Fixes #75.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 18, 2025

@ilaflott 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

…on warnings

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
Copilot AI changed the title [WIP] pkg_resources package is slated for removal as early as 2025-11-30 Replace deprecated pkg_resources with importlib.resources Sep 18, 2025
Copilot AI requested a review from ilaflott September 18, 2025 19:57
Copy link
Copy Markdown
Member

@ilaflott ilaflott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

  • update the CI to remove python 3.8 testing

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 18, 2025

@copilot

  • update the CI to remove python 3.8 testing

Updated the CI configuration to remove Python 3.8 testing and added Python 3.12 for broader coverage. Python 3.8 has reached end of life and removing it aligns with focusing on currently supported versions. Commit: 0b67657

Copilot AI requested a review from ilaflott September 18, 2025 20:04
@ilaflott ilaflott marked this pull request as ready for review September 18, 2025 20:10
Copy link
Copy Markdown
Member

@ilaflott ilaflott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup

@J-Lentz
Copy link
Copy Markdown
Contributor

J-Lentz commented Sep 22, 2025

I think this could be improved by using the pathlib.Path object returned by importlib.resources.files more idiomatically: form the path using the '/' operator, and pass the pathlib.Path object to validate_yaml rather than typecast it to a string.

validate_yaml would need to be updated, as it currently assumes (in its verboseprint calls) that the ypath and spath arguments are strings. My vote is to just delete the verboseprint calls, as well as the debug option that enables them.

Copy link
Copy Markdown
Contributor

@J-Lentz J-Lentz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address the comment above.

@ilaflott
Copy link
Copy Markdown
Member

@copilot respond to @J-Lentz feedback

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 22, 2025

@copilot please address the comment above.

Implemented the suggested improvements: now using pathlib.Path idiomatically with the / operator for path construction, passing Path objects to validate_yaml, and removed the verbose debug functionality while maintaining backward compatibility. The debug parameter is kept but ignored. Commit: a5412a7

@copilot respond to @J-Lentz feedback

Addressed @J-Lentz's feedback by implementing idiomatic pathlib.Path usage and removing debug verbosity. The code now uses importlib.resources.files('fms_yaml_tools') / "schema/..." pattern and passes Path objects to validate_yaml. Debug parameter maintained for backward compatibility but verbose output removed. Commit: a5412a7

Copilot AI changed the title Replace deprecated pkg_resources with importlib.resources Replace deprecated pkg_resources with importlib.resources and improve path handling Sep 22, 2025
Copilot AI requested a review from ilaflott September 22, 2025 18:45
@ilaflott
Copy link
Copy Markdown
Member

@copilot remove the docstring edit you added- or make it compatible with flake8 linting, which complained with:

./fms_yaml_tools/schema/validate_schema.py:60:1: W293 blank line contains whitespace
    """Validate a YAML file against a JSON schema.
    
    Args:
        ypath: Path to the YAML file to validate (string or Path object)
        spath: Path to the schema file (string or Path object)  
        debug: Deprecated parameter, kept for backward compatibility (ignored)
        success: Whether to print success message
    """
^
./fms_yaml_tools/schema/validate_schema.py:63:63: W291 trailing whitespace
    """Validate a YAML file against a JSON schema.
    
    Args:
        ypath: Path to the YAML file to validate (string or Path object)
        spath: Path to the schema file (string or Path object)  
        debug: Deprecated parameter, kept for backward compatibility (ignored)
        success: Whether to print success message
    """
                                                  
    
      ^
./fms_yaml_tools/schema/validate_schema.py:67:1: W293 blank line contains whitespace
    
^
1     W291 trailing whitespace
2     W293 blank line contains whitespace

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 22, 2025

@copilot remove the docstring edit you added- or make it compatible with flake8 linting, which complained with:

./fms_yaml_tools/schema/validate_schema.py:60:1: W293 blank line contains whites...

Fixed the flake8 whitespace issues in the docstring by removing trailing whitespace and cleaning up blank lines. The docstring is now compatible with flake8 linting and all whitespace issues have been resolved. Commit: b97c1e1

Copy link
Copy Markdown
Member

@ilaflott ilaflott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup! @J-Lentz

@ilaflott ilaflott requested a review from J-Lentz September 22, 2025 19:08
Copy link
Copy Markdown
Member

@ilaflott ilaflott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot this is very close

  • restore the definition of the verboseprint lambda function originally in fms_yaml_tools/schema/validate_schema.py line 62
  • also restore the subsequent calls to the function
  • don't change anything else

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 22, 2025

@copilot this is very close

  • restore the definition of the verboseprint lambda function originally in fms_yaml_tools/schema/validate_schema.py line 62
  • also restore the subsequent calls to the...

Restored the verboseprint lambda function definition and all subsequent calls to the function as requested. The debug functionality is now fully operational while maintaining compatibility with pathlib.Path objects through str() conversion. No other changes were made. Commit: dc773f6

@ilaflott ilaflott merged commit 71dfbd0 into main Sep 23, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pkg_resources package is slated for removal as early as 2025-11-30

3 participants