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

CI tests fail when upgrading pandas from 2.1.4 to >=2.2.0 #232

Closed
brews opened this issue Mar 27, 2024 · 7 comments
Closed

CI tests fail when upgrading pandas from 2.1.4 to >=2.2.0 #232

brews opened this issue Mar 27, 2024 · 7 comments

Comments

@brews
Copy link
Member

brews commented Mar 27, 2024

PR #216 suggests the package fails tests when the CI environment bumps pandas from v2.1.4 to the latest version (currently v2.2.1). It started failing since the pandas v2.2.0 release.

I can't judge why it's failing from a quick look at the testing logs.

Archived logs from the failed CI run are here: logs_22096399592.zip

@kemccusker
Copy link
Member

Looks like all the test_*_points tests failed. The first failure is test_adding_up_points with an AssertionError on assert_frame_equal. Looks like something is out of order somehow. The relevant log output:

2024-03-26T00:13:08.3374133Z     @pytest.mark.parametrize("menu_class", [Baseline], indirect=True)
2024-03-26T00:13:08.3374743Z     def test_adding_up_points(menu_instance, discount_types):
2024-03-26T00:13:08.3375564Z         path = f"adding_up_{discount_types}_eta{menu_instance.eta}_rho{menu_instance.rho}_damage_function_points.csv"
2024-03-26T00:13:08.3376375Z         expected = open_zipped_results(path)
2024-03-26T00:13:08.3376920Z         actual = menu_instance.damage_function_points
2024-03-26T00:13:08.3377963Z >       assert_frame_equal(expected, actual, rtol=1e-4, atol=1e-4)
2024-03-26T00:13:08.3378373Z 
2024-03-26T00:13:08.3378553Z tests/test_baseline.py:15: 
2024-03-26T00:13:08.3379103Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
2024-03-26T00:13:08.3379719Z testing.pyx:55: in pandas._libs.testing.assert_almost_equal
2024-03-26T00:13:08.3380233Z     ???
2024-03-26T00:13:08.3380710Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
2024-03-26T00:13:08.3381061Z 
2024-03-26T00:13:08.3381209Z >   ???
2024-03-26T00:13:08.3381652Z E   AssertionError: DataFrame.iloc[:, 0] (column name="ssp") are different
2024-03-26T00:13:08.3382252Z E   
2024-03-26T00:13:08.3382704Z E   DataFrame.iloc[:, 0] (column name="ssp") values are different (66.66667 %)
2024-03-26T00:13:08.3384250Z E   [index]: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, ...]
2024-03-26T00:13:08.3387749Z E   [left]:  [SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, SSP4, SSP4, SSP2, SSP2, SSP3, SSP3, ...]
2024-03-26T00:13:08.3392360Z E   [right]: [SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, SSP2, ...]
2024-03-26T00:13:08.3394861Z E   At positional index 2, first diff: SSP3 != SSP2
2024-03-26T00:13:08.3395181Z 
2024-03-26T00:13:08.3395380Z testing.pyx:173: AssertionError

@brews brews changed the title CI tests fail when upgrading pandas from 2.1.4 to >2.2.1 CI tests fail when upgrading pandas from 2.1.4 to >=2.2.0 Mar 27, 2024
@brews
Copy link
Member Author

brews commented Mar 27, 2024

Looks like it's started to fail since it tried to upgrade pandas to v2.2.0.

I updated the title and description with this info.

@JMGilbert
Copy link
Contributor

I think it's probably related to this: https://pandas.pydata.org/docs/whatsnew/v2.2.0.html#merge-and-dataframe-join-now-consistently-follow-documented-sort-behavior

@JMGilbert
Copy link
Contributor

#216 now includes a fix.

@brews
Copy link
Member Author

brews commented Apr 3, 2024

I appreciate the fix @JMGilbert!

Stop me if I'm reading this wrong but on quick glance it looks like the issue isn't with the dscim package code itself. The issue is more that the tests were written to read the expected results from an older file of stashed results. This expected results file was written with an older version of pandas and so it needs to get sorted before it can be compared with the results generated from the newer pandas version?

Looking at this ... hmm... It's not something that needs to get fixed here, but It would be ideal if the tests didn't need to handle this type of logic, instead focusing on what's needed to test some target behavior. The hacky workaround stuff makes it harder to reason what the test is actually trying to test. It's going to keep getting muddled as time marches forward. Again, I think this is absolutely something that shouldn't be addressed in this issue and PR. The code that needs updating had this hacky stuff before this issue. Depending on priorities, this is maybe something to consider for future cleanup before legacy slows things down too much, @kemccusker. It might come back to bite later.

@kemccusker
Copy link
Member

@brews would love your input on how better to write the tests to avoid these types of issues, for future cleanup. For this PR, sorting seems like a reasonable fix, yeah?

@brews
Copy link
Member Author

brews commented Apr 4, 2024

Closed by #216

@brews brews closed this as completed Apr 4, 2024
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

No branches or pull requests

3 participants