Skip to content

Fixes to prepare the code for pandas 2.0#1747

Merged
scarlehoff merged 2 commits into
masterfrom
pandas2.0_changes
May 29, 2023
Merged

Fixes to prepare the code for pandas 2.0#1747
scarlehoff merged 2 commits into
masterfrom
pandas2.0_changes

Conversation

@scarlehoff
Copy link
Copy Markdown
Member

@scarlehoff scarlehoff commented May 26, 2023

Fixes #1722, at least such that the test pass in my computer.

Note, right now we are "blocked" in pandas < 2.0 due to eko. But we might as well fix these small things as we find them. In this case they removed a keyword and change some defaults https://pandas.pydata.org/docs/reference/api/pandas.DataFrame.groupby.html

Since pandas 2.0 changed a bunch of defaults we will have to look very careful at many reports when we decide to make the change... so for now I'm happy that we are blocked at 1.X.

@scarlehoff scarlehoff requested a review from Zaharid May 26, 2023 13:43
@felixhekhorn
Copy link
Copy Markdown
Contributor

Since we decoupled banana from eko there should be no dependency to pandas left from the eko side - see here: https://github.com/NNPDF/eko/blob/master/pyproject.toml
Keep in mind that here we need the "box" extras (for evolven3fit_new), but not the "mark" extras (only used for internal benchmarking)

@scarlehoff
Copy link
Copy Markdown
Member Author

We need banana for the evolution with eko. But it doesn't matter, I don't have the energy to look through one thousands reports to check everything is fine with 2.0 :P

@felixhekhorn
Copy link
Copy Markdown
Contributor

We need banana for the evolution with eko.

Hmm why? now, not any longer ... you need genpdf which is in ekobox, no? cc @andreab1997

@scarlehoff
Copy link
Copy Markdown
Member Author

now, not any longer

Or maybe is because QED? I don't know. But it is still in the conda recipe

- banana-hep >=0.6.8

But as I said, doesn't matter at the moment!

@felixhekhorn
Copy link
Copy Markdown
Contributor

If and when you merge or not I don't mind 😛 but I want to take eko out of the game 😇 maybe the conda recipe is outdated ...

@scarlehoff scarlehoff requested a review from RoyStegeman May 29, 2023 10:53
Copy link
Copy Markdown
Member

@RoyStegeman RoyStegeman left a comment

Choose a reason for hiding this comment

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

How did you ensure this covers the complete set of changes? Did you go through the list of changes listed here: https://pandas.pydata.org/docs/dev/whatsnew/v2.0.0.html?

@scarlehoff
Copy link
Copy Markdown
Member Author

This does not. This just ensures that at least the code runs in both 2.0 and 1.X at least as far as the tests are concerned.

If we want to update to pandas 2.X we need to actually check the reports one by one since some changes might be only in their effects while the code still runs. Maybe we actually want to fix pandas<2 in the conda recipe I guess.

@RoyStegeman
Copy link
Copy Markdown
Member

That's indeed what I expected, but in that case I'd rather not make these changes yet. These are rather a minor effort and worse case we may in the future move to 2.0 without thinking about it (because the tests pass) but some behaviour changes nonetheless.

I suppose another way to prevent this is indeed to set an upper bound in the dependencies, so that would be fine with me as well.

@scarlehoff
Copy link
Copy Markdown
Member Author

I suppose another way to prevent this is indeed to set an upper bound in the dependencies, so that would be fine with me as well.

I'll do this. The reason is that I would like to move (locally) to pandas 2.0 as soon as possible so that I can fix things organically when/if I find them.

@RoyStegeman
Copy link
Copy Markdown
Member

One way is indeed to try it locally for a while, though it's not a huge list of changes that would have to be checked either. We wouldn't need to check for the deprecated or removed functions/inputs since that would result in an error and we'd notice. The things we need to be careful about is changes to defaults, but that's not such a long list to check.

@scarlehoff scarlehoff merged commit b3595a6 into master May 29, 2023
@scarlehoff scarlehoff deleted the pandas2.0_changes branch May 29, 2023 13:51
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.

Use Pandas 2.0

3 participants