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

feat: refactor var description #1443

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

vorel99
Copy link
Contributor

@vorel99 vorel99 commented Sep 14, 2023

This is my second refactoring pull request. First was #1302
My aim is to replace dictionaries with classes for easier feature developing.

  • new dataclasses for series description and series description hashable
  • add property for common propertyes from dict
  • support old dict like syntax in those classes
  • remove describe_counts and describe_generic (logic moved to classes VarDescription)

@vorel99 vorel99 marked this pull request as ready for review September 14, 2023 18:17
@vorel99 vorel99 marked this pull request as draft September 14, 2023 18:45
@vorel99
Copy link
Contributor Author

vorel99 commented Sep 14, 2023

missing spark refactoring (tbd)

@fabclmnt
Copy link
Contributor

fabclmnt commented Sep 19, 2023

Hi @vorel99 ,

Thank you for your contribution!
I'll just ask a small change, we don't use the "refactor: ". Please ensure that you use "feat: " instead or any other naming and convention that we are using for the automatic calculation of the package versions.

In what concerns the proposed changes we will be analyzing and validating wether they fit the roadmap for the package.

@vorel99 vorel99 changed the title Refactor var description feat: Refactor var description Sep 19, 2023
@aquemy aquemy changed the title feat: Refactor var description feat: refactor var description Sep 20, 2023
@aquemy
Copy link
Contributor

aquemy commented Sep 20, 2023

@vorel99 The PR looks great. Whenever you are ready, please, mark it as Ready for review such that we can start the CI/CD. I'll also do some local tests to ensure everything is fine.

@aquemy aquemy force-pushed the develop branch 2 times, most recently from 6a3342a to 8f4f622 Compare October 10, 2023 10:25
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.

None yet

3 participants