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

Make transform params stricter 2 #2177

Merged
merged 3 commits into from
Jan 4, 2023
Merged

Conversation

aesharpe
Copy link
Member

@aesharpe aesharpe commented Jan 4, 2023

Teeny tiny bb PR that adds the following code to the TransformParams class:

    class Config:
        """Only allow the known table transform params."""

        extra = "forbid"

And fixes a parameter name that was flip-flopped:

  • xbrl_instant and xbrl_duration were used as parameters in the Ferc1TableTransformParams class when the actual parameters defined in RenameColumnsFerc1 are instant_xbrl and duration_xbrl. This PR flips them to reflect the RenameColumnsFerc1 definition.

@aesharpe aesharpe added ferc1 Anything having to do with FERC Form 1 rmi xbrl Related to the FERC XBRL transition labels Jan 4, 2023
@aesharpe aesharpe self-assigned this Jan 4, 2023
@aesharpe aesharpe changed the base branch from main to dev January 4, 2023 16:43
@codecov
Copy link

codecov bot commented Jan 4, 2023

Codecov Report

Base: 85.5% // Head: 85.5% // Decreases project coverage by -0.0% ⚠️

Coverage data is based on head (8aec193) compared to base (955b95a).
Patch coverage: 100.0% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             dev   #2177     +/-   ##
=======================================
- Coverage   85.5%   85.5%   -0.1%     
=======================================
  Files         73      73             
  Lines       8883    8882      -1     
=======================================
- Hits        7600    7599      -1     
  Misses      1283    1283             
Impacted Files Coverage Δ
src/pudl/transform/ferc1.py 95.4% <ø> (-0.1%) ⬇️
src/pudl/transform/classes.py 93.4% <100.0%> (+<0.1%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@zaneselvans zaneselvans left a comment

Choose a reason for hiding this comment

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

Suggest that we remove the now redundant specification of extra = forbid in the Ferc1TableTransformParams class.

src/pudl/transform/classes.py Show resolved Hide resolved
Comment on lines -551 to +552
xbrl_instant=RenameColumns(),
xbrl_duration=RenameColumns(),
instant_xbrl=RenameColumns(),
duration_xbrl=RenameColumns(),
Copy link
Member

Choose a reason for hiding this comment

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

Eeek, we had this inconsistency with all of the actual parameters and it didn't cause any problems? That's kind of amazing.

…ts from TransformParams where extra=forbid is already an attribute
@aesharpe aesharpe merged commit bc71c5e into dev Jan 4, 2023
@aesharpe aesharpe deleted the make_transform_params_stricter_2 branch January 4, 2023 23:53
@zaneselvans zaneselvans linked an issue Jan 9, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ferc1 Anything having to do with FERC Form 1 rmi xbrl Related to the FERC XBRL transition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flag invalid params in params.py
2 participants