-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TST: move custom business month tests to own file (#27085) #42284
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
TST: move custom business month tests to own file (#27085) #42284
Conversation
can you merge master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @felixDulys. Can you please run isort in this branch, so the CI is green. And if you can make sure everything is green, and mostly updated with master, and ping us when it is, that would be great.
@github-actions pre-commit |
@datapythonista Thank you for spending the time to approve this work. I'm really hoping to get more involved in open source, but I have a lot to learn. I fixed the piece of the process I overlooked (isort) and re-merged master. Afterward, I had some CI checks fail that seemed unrelated to the changes that I made (errors while running pytest on I was able to replicate these errors locally and merging master again just now did not resolve them. How would you like for me to proceed? |
I'm in my phone now, can't check easily myself, but in general the best after what you did is to check the last build of master, and see if it's failing. If it's not, it's likely that the problem is in your branch. If master is failing, you can just wait for someone else to fix it, or if you prefer, you can also have a look, and see if you can fix it in a separate PR. |
especially if it merges an updated upstream into a topic branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm merge on green
Thanks for working on this @felixDulys |
Move custom month business cases from the
test_month.py
file into their owntest_custom_business_month.py
file per the current pattern in thetseries.offsets
tests that split offset business and/or custom subclasses into their own files with ample cases (seetest_custom_business_hour.py
)There is much refactoring work here to do that could be linked to this issue (#27085) going forward, but the simplest pieces that probably belong under this issue are simple organization of existing tests into smaller, logical files that make it easier for contributors to find a tests' home.
I'm a new contributor to the codebase so place let me know if I misunderstood the scope or what you all were thinking with this issue. Happy to adjust.