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

Fix data time accessor with persiantools #8

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mobinmohammadi10
Copy link

سلام من با استفاده از یه کتاب خانه اوپن سورس اکسسور ها رو درست کردم و تست ها همه Pass شدند

Copy link
Contributor

sourcery-ai bot commented Feb 14, 2024

This automated comment suggests enhancements to the PR title and body to improve clarity and facilitate a quicker review

Title suggestion

Fix Jalali date handling in pandas using Persiantools
Reasons to update the title
  • Consider specifying the library used and the problem it solves more clearly.
  • The title could be more descriptive about the nature of the fix.

Body suggestion

This PR addresses an issue with handling Jalali dates in pandas dataframes and series. Previously, the accessor methods were not correctly converting or managing Jalali dates. By integrating the Persiantools library, we've corrected this behavior, ensuring that Jalali dates are properly handled. All tests have passed, confirming the effectiveness of this solution. This change was motivated by the need for more robust support for Jalali dates in our project, and Persiantools was chosen for its comprehensive handling of Jalali date conversions.
Reasons to update the body
  • The description should be in English to ensure it's accessible to all contributors.
  • Explain what the problem was and how it was solved.
  • Mention why Persiantools was chosen for this fix.
  • Include any relevant links or references to documentation or issues that this PR addresses.

Benefits of a great title and description

Author benefits

  • Faster Approval Times: Clear descriptions lead to quicker, more efficient code review processes.
  • Higher Quality Reviews: Well-crafted descriptions lead to more insightful feedback, improving the overall quality of the code.
  • Easier Future Maintenance: Simplifies debugging and updating code by providing context and rationale.

Reviewer benefits

  • Efficient Review Process: Concise, informative descriptions enable quicker understanding and assessment of changes.
  • Improved Decision-Making: Detailed context aids in evaluating the impact and necessity of the change.
  • Facilitates Knowledge Sharing: Offers insights into codebase evolution, design choices, and problem-solving approaches.

Guide: Writing good PR descriptions - Google

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

PR Type: Refactoring

PR Summary: This pull request introduces a significant refactoring of the jalali_pandas package to improve its handling of Jalali dates by integrating the persiantools library. It modifies both the series and dataframe handlers to use the JalaliDateTime class from persiantools instead of the previous jdatetime library. Additionally, it includes changes to the test suite to align with these modifications and introduces a GitHub Actions workflow for automating the package publishing process upon release creation.

Decision: Comment

📝 Type: 'Refactoring' - not supported yet.
  • Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
✅ Small diff: the diff is small enough to approve with confidence.
No details provided.

General suggestions:

  • Ensure that the transition from jdatetime to persiantools' JalaliDateTime does not introduce any regressions in functionality, especially in edge cases not covered by existing tests.
  • Consider adding more comprehensive tests that specifically target the new functionality introduced by using the persiantools library, to ensure full compatibility and to catch any potential issues early.
  • Review the GitHub Actions workflow to ensure that it meets all the requirements for publishing the package, including versioning and dependency management, to avoid any deployment issues.
  • Given the significant changes, it might be beneficial to update the documentation to reflect the switch to persiantools and any new capabilities or usage patterns this enables for end-users.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@ghodsizadeh
Copy link
Owner

سلام ممنونم
میتونید بیشتر توضیح بدید که چه مشکلی رو حل کردید؟

@mobinmohammadi10
Copy link
Author

دو تا fail توی تست های ریپازیتوری قسمت تست groupby() بود که اون هارو رفع کردم و به جای jdatetime از persiantools استفاده کردم که اوپن سورس هستش که این کمک میکنه که در مراحل بعدی برای توسعه افراد بیشتری بتونن روی تاریخ ها کار کنن و همچنین اونیکی ریپو هم تکمیل تر میشه که فکر میکنم برای اوپن سورس فارسی واقعا لازم باشه.

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

2 participants