Skip to content

Conversation

sfc-gh-jkew
Copy link
Contributor

Originally the tests in test_compiler_caster.py only tested against query compilers; so checking that a cast occurred correctly was performed with an assert similar to assert type(qc1) == type(qc2). Eventually the changes were lifted up into the DataFrame object so these asserts became invalid although they continued to pass. A recent test, test_merge_in_place was added recently where I accidentally used the old pattern. While fixing this I also discovered that test_information_asymmetry was also still using the old style.

  • first commit message and PR title follow format outlined here

    NOTE: If you edit the PR title to match this format, you need to add another commit (even if it's empty) or amend your last commit for the CI job that checks the PR title to pick up the new PR title.

  • passes flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
  • passes black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
  • signed commit with git commit -s
  • Resolves TEST: Assert is wrong in test_merge_in_place #7686
  • tests added and passing
  • module layout described at docs/development/architecture.rst is up-to-date

@sfc-gh-jkew sfc-gh-jkew marked this pull request as ready for review October 1, 2025 06:17
Copy link
Contributor

@sfc-gh-mvashishtha sfc-gh-mvashishtha left a comment

Choose a reason for hiding this comment

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

We should use == for string comparisons.

@sfc-gh-jkew sfc-gh-jkew merged commit 99037af into main Oct 1, 2025
78 checks passed
sfc-gh-mvashishtha added a commit that referenced this pull request Oct 2, 2025
…ead of type (#7687)

Originally the tests in `test_compiler_caster.py` only tested against
query compilers; so checking that a cast occurred correctly was
performed with an assert similar to `assert type(qc1) == type(qc2)`.
Eventually the changes were lifted up into the DataFrame object so these
asserts became invalid although they continued to pass. A recent test,
`test_merge_in_place` was added recently where I accidentally used the
old pattern. While fixing this I also discovered that
`test_information_asymmetry` was also still using the old style.

- [x] first commit message and PR title follow format outlined
[here](https://modin.readthedocs.io/en/latest/development/contributing.html#commit-message-formatting)
> **_NOTE:_** If you edit the PR title to match this format, you need to
add another commit (even if it's empty) or amend your last commit for
the CI job that checks the PR title to pick up the new PR title.
- [x] passes `flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py`
- [x] passes `black --check modin/ asv_bench/benchmarks
scripts/doc_checker.py`
- [x] signed commit with `git commit -s` <!-- you can amend your commit
with a signature via `git commit -amend -s` -->
- [x] Resolves #7686 
- [x] tests added and passing
- [x] module layout described at `docs/development/architecture.rst` is
up-to-date <!-- if you have added, renamed or removed files or
directories please update the documentation accordingly -->

---------

Co-authored-by: Mahesh Vashishtha <[email protected]>
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.

TEST: Assert is wrong in test_merge_in_place
2 participants