Skip to content

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

Merged
merged 7 commits into from
Jul 23, 2021

Conversation

felixDulys
Copy link
Contributor

@felixDulys felixDulys commented Jun 28, 2021

Move custom month business cases from the test_month.py file into their own test_custom_business_month.py file per the current pattern in the tseries.offsets tests that split offset business and/or custom subclasses into their own files with ample cases (see test_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.

@jreback jreback added Frequency DateOffsets Testing pandas testing functions or related to the test suite labels Jul 1, 2021
@jreback
Copy link
Contributor

jreback commented Jul 1, 2021

can you merge master

@jreback jreback added this to the 1.4 milestone Jul 4, 2021
Copy link
Member

@datapythonista datapythonista left a 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.

@felixDulys
Copy link
Contributor Author

@github-actions pre-commit

@felixDulys
Copy link
Contributor Author

@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 pandas/tests/io/fast_parquet.py).

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?

@datapythonista
Copy link
Member

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.
Copy link
Contributor

@jreback jreback left a 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

@datapythonista datapythonista merged commit d2f7e98 into pandas-dev:master Jul 23, 2021
@datapythonista
Copy link
Member

Thanks for working on this @felixDulys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frequency DateOffsets Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants