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

Add IndexedFrame class and move SingleColumnFrame to a separate module #9378

Merged
merged 12 commits into from
Oct 14, 2021

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Oct 5, 2021

This PR adds a new IndexedFrame subclass of Frame. For the moment this is a minimal class with one user-facing method and the indexer properties, but as we make progress on #9038 we are likely to find more methods that should live here. Additionally, once we remove references to Frame and Index types from the Cython layer of cudf we will be able to move all index-related logic from Frame into this class and provide a suitable ducktyped interface for methods that need to behave differently for frames depending on whether or not they have an index.

In the process I also moved SingleColumnFrame into its own module. One class per file is a good organizational pattern, especially for huge classes like these, and it can also help reduce issues with circular imports and improve mypy runtime.

@vyasr vyasr added 3 - Ready for Review Ready for review by team code quality Python Affects Python cuDF API. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Oct 5, 2021
@vyasr vyasr added this to the CuDF Python Refactoring milestone Oct 5, 2021
@vyasr vyasr self-assigned this Oct 5, 2021
@vyasr vyasr requested a review from a team as a code owner October 5, 2021 16:36
@codecov
Copy link

codecov bot commented Oct 5, 2021

Codecov Report

Merging #9378 (4f8ec11) into branch-21.12 (ab4bfaa) will increase coverage by 0.00%.
The diff coverage is 0.00%.

❗ Current head 4f8ec11 differs from pull request most recent head 3efe8a2. Consider uploading reports for the commit 3efe8a2 to get more accurate results
Impacted file tree graph

@@              Coverage Diff               @@
##           branch-21.12    #9378    +/-   ##
==============================================
  Coverage         10.79%   10.79%            
==============================================
  Files               116      117     +1     
  Lines             18869    19427   +558     
==============================================
+ Hits               2036     2098    +62     
- Misses            16833    17329   +496     
Impacted Files Coverage Δ
python/cudf/cudf/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/_lib/__init__.py 0.00% <ø> (ø)
python/cudf/cudf/_lib/strings/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/csv.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/hdf.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/orc.py 0.00% <0.00%> (ø)
python/cudf/cudf/_version.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/abc.py 0.00% <0.00%> (ø)
python/cudf/cudf/api/types.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/dlpack.py 0.00% <0.00%> (ø)
... and 60 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cff71ff...3efe8a2. Read the comment docs.

@@ -500,95 +499,6 @@ def _explode(self, explode_column: Any, ignore_index: bool):
res.index.names = self._index.names
return res

def _sort_index(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An FYI for reviewers: This method is the result of a bad rebase of mine on a previous PR. It is redundant (replaced by the non-underscored sort_index method below) and safe to remove.

@vyasr
Copy link
Contributor Author

vyasr commented Oct 14, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 8a18d7a into rapidsai:branch-21.12 Oct 14, 2021
@vyasr vyasr deleted the refactor/indexed_frame branch January 14, 2022 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants