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

deprecation of _indices #69

Merged
merged 15 commits into from
Aug 15, 2024
Merged

deprecation of _indices #69

merged 15 commits into from
Aug 15, 2024

Conversation

sjsrey
Copy link
Member

@sjsrey sjsrey commented Apr 24, 2024

xref #67

inequality/_indices.py Outdated Show resolved Hide resolved
Co-authored-by: James Gaboardi <[email protected]>
@jGaboardi
Copy link
Member

Should we disable doctests for the _indices.py module to get CI green?

inequality/_indices.py Outdated Show resolved Hide resolved
inequality/_indices.py Outdated Show resolved Hide resolved
@jGaboardi
Copy link
Member

@sjsrey shall we pick this back up again to get CI green?

Co-authored-by: James Gaboardi <[email protected]>
@martinfleis
Copy link
Member

Slight issue I see is that this warning is emitted by import inequality even if you're not interacting with the indices module. We may need to emit it in the functions themselves. Maybe a decorator?

@jGaboardi
Copy link
Member

Looks like have a fresh exception being thrown by bleeding edge pandas now. If it's not something to simply let ride, we can open a new ticket for it.

201     >>> import libpysal
UNEXPECTED EXCEPTION: ImportError("cannot import name 'CachedAccessor' from 'pandas.core.accessor' (/home/runner/micromamba/envs/test/lib/python3.12/site-packages/pandas/core/accessor.py)")
Traceback (most recent call last):
  File "/home/runner/micromamba/envs/test/lib/python3.12/doctest.py", line 1361, in __run
    exec(compile(example.source, filename, "single",
  File "<doctest inequality.theil.TheilDSim[0]>", line 1, in <module>
  File "/home/runner/micromamba/envs/test/lib/python3.12/site-packages/libpysal/__init__.py", line 27, in <module>
    from . import cg, examples, io, weights
  File "/home/runner/micromamba/envs/test/lib/python3.12/site-packages/libpysal/cg/__init__.py", line 12, in <module>
    from .voronoi import *
  File "/home/runner/micromamba/envs/test/lib/python3.12/site-packages/libpysal/cg/voronoi.py", line 10, in <module>
    import geopandas as gpd
  File "/home/runner/micromamba/envs/test/lib/python3.12/site-packages/geopandas/__init__.py", line 4, in <module>
    from geopandas.geodataframe import GeoDataFrame
  File "/home/runner/micromamba/envs/test/lib/python3.12/site-packages/geopandas/geodataframe.py", line 8, in <module>
    from pandas.core.accessor import CachedAccessor
ImportError: cannot import name 'CachedAccessor' from 'pandas.core.accessor' (/home/runner/micromamba/envs/test/lib/python3.12/site-packages/pandas/core/accessor.py)
/home/runner/work/inequality/inequality/inequality/theil.py:201: UnexpectedException

@jGaboardi
Copy link
Member

Should we disable doctests for the _indices.py module to get CI green?

We still need to address these failures, either by skipping or updating.

@martinfleis
Copy link
Member

@jGaboardi that is fixed by geopandas/geopandas#3288

@jGaboardi jGaboardi mentioned this pull request Aug 14, 2024
@sjsrey
Copy link
Member Author

sjsrey commented Aug 14, 2024

The remaining failure is related to codecov on macos-13.

@jGaboardi any suggestions?

Copy link

codecov bot commented Aug 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.2%. Comparing base (eb15ef6) to head (9a28ef2).
Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main     #69     +/-   ##
=======================================
+ Coverage   98.0%   98.2%   +0.2%     
=======================================
  Files          4       4             
  Lines        301     334     +33     
=======================================
+ Hits         295     328     +33     
  Misses         6       6             
Files Coverage Δ
inequality/_indices.py 100.0% <100.0%> (ø)
inequality/gini.py 92.6% <ø> (ø)

@jGaboardi
Copy link
Member

jGaboardi commented Aug 14, 2024

Seems that it's actually pre-commit that's complaining. Did you run ruff check inequality locally before pushing?

@jGaboardi
Copy link
Member

pre-commit.ci run

@jGaboardi
Copy link
Member

pre-commit.ci autofix

@jGaboardi
Copy link
Member

The bot is having trouble auto fixing the import sorting for some reason. Running ruff check inequality --fix locally should resolve the issue.

@jGaboardi
Copy link
Member

pre-commit.ci autofix

@jGaboardi jGaboardi merged commit a2fe5ee into pysal:main Aug 15, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants