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

Remove attribute-style accesses to (Geo)DataFrames columns and xarray Datasets variables and attributes #939

Merged
merged 14 commits into from
Sep 9, 2024

Conversation

luseverin
Copy link
Collaborator

@luseverin luseverin commented Aug 18, 2024

Changes proposed in this PR:

  • Removal of attribute style-accesses to columns of pandas DataFrames, GeoDataFrames, and variables and attributes of xarray DataSets and DataArray.
  • Mention this policy in the climada convention guide

This PR fixes SCRUM userstory 100: Avoid attribute-style access to DataFrame columns (https://tree.taiga.io/project/emanuel-schmid-climada/us/100).

PR Author Checklist

PR Reviewer Checklist

@luseverin
Copy link
Collaborator Author

luseverin commented Aug 18, 2024

I did my best to replace all instances of attribute-style accesses with the recommended keyword-style accesses. Please let me know if you spot some instances that I missed!

What I still need to do is to update tests (as I assume this needs to be done as well).

@luseverin luseverin added good first issue conventions Coding conventions or style labels Aug 19, 2024
import os
import sys

class DataFrameAttributeVisitor(ast.NodeVisitor):
Copy link
Member

Choose a reason for hiding this comment

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

What is the goal of this class?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, it got mistakenly commited, I will remove it. It was a script to try to automatically find out occurences of attribute-style accesses but it was not working out well..

@spjuhel
Copy link
Collaborator

spjuhel commented Aug 21, 2024

Great job!

I looked around with different regexps based on yours:

  • There are some "exp.gdf.value" in util/lines_polys_handler.py (in docstrings mainly)
  • entity/measure/base.py:492
  • engine/unsequa/unc_output.py:1028

I did not find others (except for tests, but you said you still haven't done these yet)

@luseverin
Copy link
Collaborator Author

Ok so I removed the few instances that I missed + the ones in the tests. Everything should be covered now. However I now have some tests failing with some weird errors e.g. AttributeError: module 'importlib_metadata' has no attribute 'entry_points' . I am not sure how this relates to my changes. When I did the make unit_test and make integ_test everything was passing.. Any idea where this might come from?

Copy link
Collaborator

@spjuhel spjuhel left a comment

Choose a reason for hiding this comment

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

Very good job! You can merge as soon as we understand why the test are failing on Jenkins.

@emanuel-schmid any idea?

@emanuel-schmid
Copy link
Collaborator

awesome! many thanks!

(the importlib_metadata error was most likely just about some incompatible conda package)

@emanuel-schmid emanuel-schmid merged commit 2ba8be9 into develop Sep 9, 2024
18 checks passed
@emanuel-schmid emanuel-schmid deleted the feature/remove_attribute-style_accesses branch September 9, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conventions Coding conventions or style good first issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants