[N/A] New features are documented, with examples if plot related.
[N/A] New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
[N/A] API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).
[N/A] Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).

oscargus added the third-party integration: pandas label

Feb 24, 2022

greglucas reviewed

Feb 25, 2022

View reviewed changes

lib/matplotlib/axes/_axes.py Outdated

		if hasattr(X, 'values'): # support pandas.Series
		X = X.values
		if hasattr(X, 'to_numpy'): # support pandas.Series
		X = X.to_numpy()

Copy link

Contributor

Choose a reason for hiding this comment

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

Are there any libraries we need to worry about that implementX.values, but notX.to_numpy()? Maybe we also want to leave anelif with theX.values block as a fallback just in case...?

Copy link

Member

QuLogic commentedFeb 25, 2022

Perhaps should be coordinated with#22141?

Copy link

MemberAuthor

oscargus commentedFeb 25, 2022•
edited
Loading

Perhaps should be coordinated with#22141?

That's where I got the idea. As seen, I have not touchedcbook.

Related to the question from@greglucas, I do not know. But I saw this code incbook (which I didn't touch):

matplotlib/lib/matplotlib/cbook/__init__.py

Lines 1369 to 1377 in3a994d2

	# unpack if we have a values or to_numpy method.
	try:
	X=X.to_numpy()
	exceptAttributeError:
	try:
	ifisinstance(X.values,np.ndarray):
	X=X.values
	exceptAttributeError:
	pass

This is a bit more complex as it uses a try-except approach (not sure how much that affects things though), has a fallback on values and check that values actually returns an nparray. One can of course use a similar approach here (and in#22141). Possibly slightly improved as I do not know if there are cases if values actually is non-trivial, so no need to run it twice.

Edit: I have now touched this and use a similar approach, but withhasattr instead oftry-except seehttps://stackoverflow.com/questions/903130/hasattr-vs-try-except-block-to-deal-with-non-existent-attributes as the assumption is that most often we do not pass pandas or xarray (or whatever) data.

Copy link

Contributor

greglucas commentedFeb 25, 2022

I guess you could write acbook._unpack_pandas(x) helper to use that in all of these areas for consistency? I would say it is safer to keep the.values fallback, and if you do want to get rid of it eventually, you could put in a deprecation warning if that branch is hit.

Copy link

MemberAuthor

oscargus commentedFeb 25, 2022

Yes, I was also thinking about a helper function (but didn't know where to place it, so great info that cbook is the place). I do not have any strong opinions as such, more that I read the link and it seemed like the right thing to do.

oscargus marked this pull request as draft

February 25, 2022 16:46

oscargus force-pushed thepandasconversion branch froma0488df to64a301eCompare

February 27, 2022 12:10

oscargus marked this pull request as ready for review

February 27, 2022 12:11

oscargus mentioned this pull request

Feb 27, 2022

Fix check 1d#22141

Merged

6 tasks

Copy link

MemberAuthor

oscargus commentedFeb 27, 2022

Seems like the easiest way is to wait for#22141, add those conversions here as well, and then discuss the correct name for the function.

oscargus added the status: waiting for other PR label

Feb 27, 2022

Copy link

MemberAuthor

oscargus commentedFeb 27, 2022

I'm thinking that maybe_unpack_to_numpy is a general enough name?

oscargus force-pushed thepandasconversion branch from64a301e toa1757d1Compare

February 28, 2022 10:23

Copy link

MemberAuthor

oscargus commentedFeb 28, 2022•
edited
Loading

This is now updated:

renamed function to_unpack_to_numpy
added test for xarray~~(now an extra test dependency)~~ (Removed extra dependency)
~~(removed duplicate test code for pandas)~~ (No duplicate code to start with...)

oscargus force-pushed thepandasconversion branch froma1757d1 to873dff2Compare

February 28, 2022 10:53

oscargus changed the title~~Improve Pandas conversion~~Improve pandas/xarray/... conversion

Feb 28, 2022

oscargus force-pushed thepandasconversion branch from873dff2 to1e27b8aCompare

February 28, 2022 11:06

oscargus mentioned this pull request

Feb 28, 2022

review how we duck-type non-numpy inputs#16402

Open

tacaswell added this to thev3.5.2 milestone

Mar 3, 2022

Copy link

Member

tacaswell commentedMar 3, 2022

Please update the lines touched by#22141

oscargus force-pushed thepandasconversion branch 3 times, most recently from19e86ca tof17f3afCompare

March 3, 2022 22:13

Copy link

MemberAuthor

oscargus commentedMar 3, 2022•
edited
Loading

There is also this function where it doesn't work to simply replaceto_numpy with_unpack_to_numpy. No idea why. (One reason may be that there is no attribute error forto_numpy anymore, so some types of input data is handled well by_check_1d, has an index method, but noto_numpy method... Seems like plain Python lists can be a culprit here then...)

matplotlib/lib/matplotlib/cbook/__init__.py

Lines 1607 to 1639 in0359832

	defindex_of(y):
	"""
	A helper function to create reasonable x values for the given y.

	This is used for plotting (x, y) if x values are not explicitly given.

	First try ``y.index`` (assuming y is a `pandas.Series`), if that
	fails, use ``range(len(y))``.

	This will be extended in the future to deal with more types of
	labeled data.

	Parameters
	----------
	y : float or array-like

	Returns
	-------
	x, y : ndarray
	The x and y values to plot.
	"""
	try:
	returny.index.to_numpy(),y.to_numpy()
	exceptAttributeError:
	pass
	try:
	y=_check_1d(y)
	except (np.VisibleDeprecationWarning,ValueError):
	# NumPy 1.19 will warn on ragged input, and we can't actually use it.
	pass
	else:
	returnnp.arange(y.shape[0],dtype=float),y
	raiseValueError('Input could not be cast to an at-least-1D NumPy array')

tacaswell approved these changes

Mar 3, 2022

View reviewed changes

tacaswell removed the status: waiting for other PR label

Mar 3, 2022

greglucas reviewed

Mar 11, 2022

View reviewed changes

lib/matplotlib/cbook/__init__.pyShow resolvedHide resolved

lib/matplotlib/tests/test_cbook.py

		np.testing.assert_array_equal(Idx, IdxRef)


		def test_index_of_xarray(xr):

Copy link

Contributor

greglucasMar 11, 2022

Choose a reason for hiding this comment

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

Does xarray get us more coverage here? They have ato_numpy() method the same as pandas I believe.
https://xarray.pydata.org/en/stable/generated/xarray.DataArray.to_numpy.html

So, it seems like a pretty heavy dependency to add for just this one test...

Copy link

MemberAuthor

oscargusMar 11, 2022

Choose a reason for hiding this comment

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

Not so much for coverage as for actually testing using data of specified formats. With the discussion about which formats we support, it makes sense to test them as well. Right now some of these are tested in the plots, but it can possibly make sense to simply test them here as these are the core function used to get data that can be plotted.

If we claim (which we actually don't, maybe we should?) that we can plot xarray, we should probably test it as well. And other types that we may want to claim to support. Or maybe fork off a specific dependency test that is not executed on all platforms/version, including pandas (which is 11.7 MB, xarray is 870 kB).

(There is another xarray-test above, so two.)

I can of course remove them, but I think we should discuss if we want to support more formats than pandas and numpy (and Python list/tuple), and, if so, have explicit tests for them.

Copy link

Contributor

greglucasMar 11, 2022

Choose a reason for hiding this comment

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

I agree, we should probably discuss what we want to support/test. To me, this doesn't seem to add a whole lot of value for adding a new dependency.

There was also a discussion around removing Scipy as a dependency in the docs:#22120

Copy link

MemberAuthor

oscargusMar 14, 2022

Choose a reason for hiding this comment

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

I removed the dependencies but kept the tests. Hence, they will run if xarray is available.

I also opened#22645 for discussions (probably should be discussed at a dev-call as well).

oscargus force-pushed thepandasconversion branch from05e8501 tof17f3afCompare

March 11, 2022 08:31

oscargus mentioned this pull request

Mar 14, 2022

[ENH]: Which array libraries should Matplotlib support (and test support for)?#22645

Open

Improve pandas and xarray conversion

7b51044

oscargus force-pushed thepandasconversion branch fromf17f3af to7b51044Compare

March 14, 2022 11:29

CI: add xarray to extra dependencies

56af810

Copy link

Member

tacaswell commentedApr 13, 2022

I made an executive decision to install xarray on CI.

We already have all of its dependencies installed and it is a pure-python package.

timhoffm reviewed

Apr 17, 2022

View reviewed changes

lib/matplotlib/cbook/__init__.py



		def _unpack_to_numpy(x):
		"""Internal helper to extract data from e.g. pandas and xarray objects."""

Copy link

Member

timhoffmApr 17, 2022

Choose a reason for hiding this comment

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

Please document what we intend to support, i.e. everything with .to_numpy() or .values, and what types we expect to catch with it, e.g. values-> older pandas dataframes(?).

jklymak merged commit709fba8 intomatplotlib:main

Apr 21, 2022

Copy link

Member

tacaswell commentedApr 21, 2022

We discussed this on the call, I will write up an issues shortly about the documentation of the intention etc.

Copy link

Member

QuLogic commentedApr 21, 2022

meeseeksdev backport to v3.5.x

meeseeksmachine mentioned this pull request

Apr 21, 2022

Backport PR #22560 on branch v3.5.x (Improve pandas/xarray/... conversion)#22876

Merged

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request

Apr 21, 2022

Backport PRmatplotlib#22560: Improve pandas/xarray/... conversion

de4fda5

QuLogic mentioned this pull request

Apr 21, 2022

bot not responding to backport label?scientific-python/MeeseeksDev#84

Closed

tacaswell mentioned this pull request

Apr 22, 2022

Document and test what "array like" means to Matplotlib#22879

Open

QuLogic added a commit that referenced this pull request

Apr 29, 2022

Merge pull request#22876from meeseeksmachine/auto-backport-of-pr-22…

3420565

…560-on-v3.5.xBackport PR#22560 on branch v3.5.x (Improve pandas/xarray/... conversion)

mocquin added a commit to mocquin/physipy that referenced this pull request

May 7, 2022

to_numpy seems not needed anymore

74057c2

seematplotlib/matplotlib#22973,matplotlib/matplotlib#22879, andmatplotlib/matplotlib#22560It is not clear to me as to which is the standard interface for unit handling (for eg, hist still doesn't handle unit by default)

Labels

third-party integration: pandas

6 participants

Movatterモバイル変換

Uh oh!

Improve pandas/xarray/... conversion#22560

Improve pandas/xarray/... conversion#22560

Uh oh!

Conversation

oscargus commentedFeb 24, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

greglucasFeb 25, 2022

Choose a reason for hiding this comment

Uh oh!

QuLogic commentedFeb 25, 2022

Uh oh!

oscargus commentedFeb 25, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

greglucas commentedFeb 25, 2022

Uh oh!

oscargus commentedFeb 25, 2022

Uh oh!

oscargus commentedFeb 27, 2022

Uh oh!

oscargus commentedFeb 27, 2022

Uh oh!

oscargus commentedFeb 28, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

tacaswell commentedMar 3, 2022

Uh oh!

oscargus commentedMar 3, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

Uh oh!

greglucasMar 11, 2022

Choose a reason for hiding this comment

Uh oh!

oscargusMar 11, 2022

Choose a reason for hiding this comment

Uh oh!

greglucasMar 11, 2022

Choose a reason for hiding this comment

Uh oh!

oscargusMar 14, 2022

Choose a reason for hiding this comment

Uh oh!

tacaswell commentedApr 13, 2022

Uh oh!

timhoffmApr 17, 2022

Choose a reason for hiding this comment

Uh oh!

tacaswell commentedApr 21, 2022

Uh oh!

QuLogic commentedApr 21, 2022

Uh oh!

Uh oh!

oscargus commentedFeb 24, 2022•
edited
Loading

oscargus commentedFeb 25, 2022•
edited
Loading

oscargus commentedFeb 28, 2022•
edited
Loading

oscargus commentedMar 3, 2022•
edited
Loading