Skip to content

Conversation

PhilippVerpoort
Copy link

@PhilippVerpoort PhilippVerpoort commented Jun 6, 2025

Passing the classifying variables and keys as arguments to the Table.get_data() function no longer works. The changes in this PR add this option back.

Here is an example:

from pystatis import Table

t = Table(name='51000-0014')
t.get_data(startyear=2019, endyear=2024, classifyingvariable1='WAM8', classifyingkey1='WA28141000,WA28142000')

This used to work in previous versions of pystatis, it stopped working at some point, and this PR tries to bring back that functionality.

This is related to issue #157.

Summary by CodeRabbit

  • New Features
    • Added support for up to three pairs of classifying variables and keys when retrieving data, allowing for more detailed and flexible data requests.

Copy link

coderabbitai bot commented Jun 6, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The get_data method of the Table class in src/pystatis/table.py was updated to accept up to three pairs of new optional string parameters for classifying variables and keys. These parameters are added to the data request if provided, allowing more flexible data queries.

Changes

File(s) Change Summary
src/pystatis/table.py Extended get_data method signature with six new optional parameters for classifying variables/keys; incorporated these into request params if set.

Poem

In the land of data, new fields bloom bright,
Classifying variables join the flight.
Three pairs now hop along,
Making queries robust and strong.
A rabbit’s cheer for options anew—
More ways to fetch, more ways to view!
🐇✨

✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/pystatis/table.py (1)

151-162: LGTM! Consider reducing repetition.

The parameter processing logic is correct and will properly add the classifying variables and keys to the API request when provided. The conditional checks ensure only non-empty values are included.

Consider refactoring to reduce repetition and improve maintainability:

-        if classifyingvariable1:
-            params['classifyingvariable1'] = classifyingvariable1
-        if classifyingkey1:
-            params['classifyingkey1'] = classifyingkey1
-        if classifyingvariable2:
-            params['classifyingvariable2'] = classifyingvariable2
-        if classifyingkey2:
-            params['classifyingkey2'] = classifyingkey2
-        if classifyingvariable3:
-            params['classifyingvariable3'] = classifyingvariable3
-        if classifyingkey3:
-            params['classifyingkey3'] = classifyingkey3
+        # Add classifying variables and keys if provided
+        classifying_params = {
+            'classifyingvariable1': classifyingvariable1,
+            'classifyingkey1': classifyingkey1,
+            'classifyingvariable2': classifyingvariable2,
+            'classifyingkey2': classifyingkey2,
+            'classifyingvariable3': classifyingvariable3,
+            'classifyingkey3': classifyingkey3,
+        }
+        params.update({k: v for k, v in classifying_params.items() if v})
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9beb108 and 23c1489.

📒 Files selected for processing (1)
  • src/pystatis/table.py (2 hunks)

@pmayd pmayd changed the base branch from main to dev June 6, 2025 18:22
Copy link

codecov bot commented Jun 6, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 80.26%. Comparing base (9beb108) to head (23c1489).
Report is 2 commits behind head on dev.

Files with missing lines Patch % Lines
src/pystatis/table.py 50.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #188      +/-   ##
==========================================
- Coverage   80.89%   80.26%   -0.63%     
==========================================
  Files          12       12              
  Lines         581      593      +12     
==========================================
+ Hits          470      476       +6     
- Misses        111      117       +6     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@pmayd pmayd left a comment

Choose a reason for hiding this comment

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

just a small correction, and pls add the new parameters to the docstring Args section so they are properly documented

"format": "ffcsv",
}

if classifyingvariable1:
Copy link
Collaborator

Choose a reason for hiding this comment

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

you actually don't need to check the variable here, it doesn't hurt to add empty variables this will be just ignored by the API

"stand": stand,
"language": language,
"quality": quality,
"format": "ffcsv",
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can directly add the parameters here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Updated

@pmayd
Copy link
Collaborator

pmayd commented Jun 6, 2025

to pass the ci you need to run uv run ruff format to format the file properly. This is also part of the pre-commit hooks ,so you can also use uv run pre-commit init and uv run pre-commit run --all

@MarcoHuebner MarcoHuebner requested a review from pmayd September 14, 2025 10:36
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.

3 participants