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

filters: use importlib_resources API to avoid deprecation warning #117

Merged
merged 4 commits into from Mar 5, 2024

Conversation

polariton
Copy link
Contributor

Fix deprecation warning "pkg_resources is deprecated as an API"

@bmcfee
Copy link
Owner

bmcfee commented Dec 19, 2023

Thanks - looks like you need to add importlib_resources to the dependencies to get CI working though.

@polariton
Copy link
Contributor Author

OK, added importlib_resources to CI.

@bmcfee
Copy link
Owner

bmcfee commented Dec 23, 2023

This should be done via package requirements, not by changing the test environment script.

@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.22%. Comparing base (1d1a08a) to head (0eddd89).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #117   +/-   ##
=======================================
  Coverage   75.22%   75.22%           
=======================================
  Files           4        4           
  Lines         113      113           
=======================================
  Hits           85       85           
  Misses         28       28           
Flag Coverage Δ
unittests 75.22% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@takenori-y
Copy link

@bmcfee
Sorry to jump in, but will this PR be merged in the near future?

Copy link
Contributor

@avalentino avalentino left a comment

Choose a reason for hiding this comment

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

LGTM

@bmcfee
Copy link
Owner

bmcfee commented Mar 4, 2024

Sorry, this fell off my radar. It looks like the tests are failing with this PR, but I'm not sure why they're not showing up in the GH actions checks here.

@bmcfee
Copy link
Owner

bmcfee commented Mar 4, 2024

https://pipelinesghubeus26.actions.githubusercontent.com/bUxMb2yqtuvB2RTCARL2QpDTvLpo5Yf3UEj1GdiJq0K3dNEhMw/_apis/pipelines/1/runs/8/signedlogcontent/3?urlExpires=2024-03-04T19%3A26%3A50.4934762Z&urlSigningMethod=HMACV1&urlSignature=wFq6a%2F9ay5mg0BrMgePItMgMpWkGm0bjH2TKOSBowcM%3D is the failed build log on 3.6. (Not that I'm too fussed with maintaining for 3.6, but if we're going to put out a release to support newer python, we shouldn't break the old version if we don't have to.)

Failures on 3.10+ environments were due to codecov being flakey and nothing with our test suite directly.

@takenori-y
Copy link

I could not see the log, but the variable __name__ seems to be different in python 3.6 and 3.7 and later.
How is this?

if filter_name not in FILTER_CACHE:
    package = __name__.split(".")[0]  # or simply 'resampy'
    fname = importlib_resources.files(package).joinpath(
        'data', os.path.extsep.join([filter_name, 'npz'])
    )

resampy/filters.py Outdated Show resolved Hide resolved
fixing package resource locations
@bmcfee
Copy link
Owner

bmcfee commented Mar 5, 2024

I just did the simple thing and hacked the package name directly in. Let's see if this makes CI happy.

@bmcfee
Copy link
Owner

bmcfee commented Mar 5, 2024

Alright, good enough. I think we still need to do a bit of tweaking on the package setup.cfg, but this is functional for now. I'll try to put a release out this week.

@bmcfee bmcfee merged commit cc6eed5 into bmcfee:main Mar 5, 2024
7 of 9 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.

None yet

5 participants