support for updating axis ticks for categorical data #6889

story645 · 2016-08-03T09:01:57Z

Now supports updating axis:

fig, ax = plt.subplots()
ax.plot(['a', 'c', 'e'], label="plot 1")
ax.plot(['a', 'b', 'd'], label="plot 2")
ax.plot(['b', 'e', 'd'], label="plot3")
ax.legend()

Which as a sidenote, it didn't work before because apparently FixedLocator converts to an array as soon as it's passed in (and also apparently it doesn't need to...)

UnitData was also changed into an object to facilitate the updating and as a precursor to adding more functionality. And the tests should now be py.test compliant @Kojoley

story645 · 2016-08-03T14:28:04Z

lib/matplotlib/category.py

+    def update(self, new_data):
+        self._set_seq(new_data)
+        value = max(self.locs)
+        self._set_locs(value + 1)


needs to be max(value +1, 0) to not conflict with spdict conventions

story645 · 2016-08-03T15:16:42Z

I may have to spin off a PR on supporting both pytest and nosetest on appveyor/travis if I ever want my tests to pass again - help @Kojoley?

Kojoley · 2016-08-03T17:32:29Z

I may have to spin off a PR on supporting both pytest and nosetest on appveyor/travis if I ever want my tests to pass again.

Unfortunately, yes.

It is nice to hear that there is a demand in pytest, and I am sorry for my slowness.
I am close to finish with pytest support, but currently there is a major issue in matplotlib that prevents running pytest with xdist (I have not opened issue for it yet, but I am working on solution).
Anyway I think nose should live for some time with pytest until we are sure that there are no false positives.

You can rebase on top of #6730 for pytest support (but it is a bit out of sync, and you need to remove -n $PROC from .travis.yml to disable xdist).

tacaswell · 2016-08-04T03:21:35Z

I would like to keep this PR mergeable independent of the pytest PR

@Kojoley You are making more progress on this in the last few weeks than we have made in the last ~ year

story645 · 2016-08-04T03:32:27Z

then what should I do? I'd rather not have to rewrite all my tests only to rewrite them again, but travis and appveyor won't work with them.

@Kojoley does it make sense to factor out just the py.test & nosetest can both run stuff out of your PR into a separate pull request? I don't want to step on your toes/your PR, but it also may make it easier for other people to adopt py.test in their testing and that not have to wait on full py.test conversion.

tacaswell · 2016-08-04T03:41:46Z

maybe pull the tests out, merge this to master with no CI (but running the tests locally), and the put a PR on top of #6730 adding the tests there?

story645 · 2016-08-04T03:52:08Z

Ugh, 'specially as this will only get worse as I write more tests and my coverage will sink really badly. I really think a relatively small PR that just lets nose and pytest work together might be a better solution unless @Kojoley has an argument against.

Kojoley · 2016-08-04T08:57:00Z

maybe pull the tests out, merge this to master with no CI (but running the tests locally), and the put a PR on top of #6730 adding the tests there?

I agree. We can comment out this line or make a new list of pytest only tests.

my coverage will sink really badly

You can still look at the coverage locally.

I don't want to step on your toes/your PR, but it also may make it easier for other people to adopt py.test in their testing and that not have to wait on full py.test conversion.

My primary goal is to make current test suite both nose & pytest compatible with minimal changes. Just do not use raise KnownFailureTest (I should separate PR for knownfailed function from #6730) and it will work with pytest without changes.

story645 · 2016-08-04T14:29:55Z

or make a new list of pytest only tests.
I vote for doing that. Also I really really don't want to have to separate my tests from my code. So is their go ahead from you to spin off a PR just for running both nose and py.test?

Just do not use raise KnownFailureTest
already doing that

Kojoley · 2016-08-04T14:37:30Z

So is their go ahead from you to spin off a PR just for running both nose and py.test?

It will be in the same PR. I have just temporary disabled nose because otherwise travis and appveyor will run hundred times and spend hours of my life, forcing me to wait for nose while the results does not matter, because all the places that touches nose already tested many times and I want simply know is it all right with pytest or not.

story645 · 2016-08-04T14:51:26Z

It will be in the same PR.

I think we may be on different pages here. I'm proposing really small PR that just adds support for py.test (nose can still do it's thing) that can hopefully be merged into master fairly quickly. My reason for proposing it is lets people like me write py.test code without having to wait on the full conversion PR getting merged in.

Kojoley · 2016-08-04T16:35:54Z

without having to wait on the full conversion PR getting merged in

PR #6730 is only about pytest compatibility with current test suit. It will not growth beyond that. The full conversion is something that I do not expect in the near future.

I think we can do following thing:

I will prepare Add Py.test testing framework support #6730 ASAP with just single-threaded support and pytest-only tests filter implementation
You will rebase this PR on top of it just for check
If all is ok, merge Add Py.test testing framework support #6730 to master, rebase this PR on top of master and merge

But I should warn you about unexpected problems. I really hope #6899 is the last stopper for pytest, but I cannot guarantee this. So if you can wait until weekend with this PR I think it will be the best choice.

story645 · 2016-08-04T17:09:15Z

Sounds good to me, and can totally wait on the weekend. Thanks

tacaswell · 2016-08-21T02:25:37Z

@story645 What is going on with this branch? This mostly rebases cleanly on to current master (see the category branch on my fork).

tacaswell · 2016-08-21T02:32:10Z

tox.ini

@@ -4,7 +4,7 @@
 # and then run "tox" from this directory.

 [tox]
-envlist = py26, py27, py31, py32


As long as you are touching this, can you remove 2.6, 3.1 and 3.2 and add 3.4?

story645 · 2016-08-21T03:15:53Z

Waiting on #6730 , or does #6920 supercede that @Kojoley

Kojoley · 2016-08-21T12:43:42Z

No, #6920 does not supercede #6730.

Kojoley · 2016-08-21T19:37:17Z

@story645 please remove the test from default_test_modules var.

story645 · 2016-08-23T16:26:10Z

Passes travis mostly 'cause this new code is no longer tested as part of this PR. But, all these tests are in #6934 anyway.

tacaswell · 2016-08-23T16:31:03Z

.mailmap

@@ -1,4 +1,5 @@
 Adrien F. Vincent <vincent.adrien@gmail.com>
+<<<<<<< 3d31c694aa6b77b9d8b7b1897f3a67f5be1a54ea


merge conflict flags

Ugh, it lied and told me there were no conflicts. fixed now.

tacaswell · 2016-08-23T17:10:00Z

@story645 this needs a rebase (again). I suspect it is in the travis config files.

…egory from init, and updated tox

story645 · 2016-08-23T17:22:24Z

Yup, travis and appveyor and the like 'cause of #6730, updated now.

story645 · 2016-08-23T20:36:04Z

Pretty sure coverage failure isn't my fault since coverage is up/neutral on the files that are part of this PR.

Kojoley · 2016-08-24T10:52:07Z

I have opened a PR #6974 for the issue

story645 added status: needs review topic: categorical labels Aug 3, 2016

story645 reviewed Aug 3, 2016
View reviewed changes

tacaswell added this to the 2.1 (next point release) milestone Aug 4, 2016

story645 mentioned this pull request Aug 10, 2016

[WIP] Categorical Color Mapping #6934

Closed

5 tasks

story645 force-pushed the category branch 2 times, most recently from b726feb to a8eac2c Compare August 17, 2016 11:03

tacaswell reviewed Aug 21, 2016
View reviewed changes

story645 force-pushed the category branch 2 times, most recently from c7cb09a to 3a8ec8e Compare August 23, 2016 14:54

tacaswell reviewed Aug 23, 2016
View reviewed changes

story645 force-pushed the category branch from 910460e to 79a62a7 Compare August 23, 2016 16:46

tacaswell closed this Aug 23, 2016

tacaswell reopened this Aug 23, 2016

tacaswell added status: needs review and removed status: needs review labels Aug 23, 2016

py.test, updating axis ticks, and unitData as class, removed test_cat…

f24330d

…egory from init, and updated tox

story645 force-pushed the category branch from 79a62a7 to f24330d Compare August 23, 2016 17:21

story645 mentioned this pull request Aug 23, 2016

Include test files in coverage report #6902

Closed

tacaswell merged commit ee8f448 into matplotlib:master Aug 25, 2016

tacaswell removed the status: needs review label Aug 25, 2016

story645 deleted the category branch March 7, 2017 01:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support for updating axis ticks for categorical data #6889

support for updating axis ticks for categorical data #6889

story645 commented Aug 3, 2016

story645 Aug 3, 2016

story645 commented Aug 3, 2016 •

edited

Loading

Kojoley commented Aug 3, 2016

tacaswell commented Aug 4, 2016

story645 commented Aug 4, 2016 •

edited

Loading

tacaswell commented Aug 4, 2016

story645 commented Aug 4, 2016

Kojoley commented Aug 4, 2016

story645 commented Aug 4, 2016

Kojoley commented Aug 4, 2016

story645 commented Aug 4, 2016 •

edited

Loading

Kojoley commented Aug 4, 2016

story645 commented Aug 4, 2016

tacaswell commented Aug 21, 2016

tacaswell Aug 21, 2016

story645 Aug 21, 2016

story645 commented Aug 21, 2016

Kojoley commented Aug 21, 2016

Kojoley commented Aug 21, 2016

story645 commented Aug 23, 2016

tacaswell Aug 23, 2016

story645 Aug 23, 2016

tacaswell commented Aug 23, 2016

story645 commented Aug 23, 2016

story645 commented Aug 23, 2016

Kojoley commented Aug 24, 2016

		@@ -1,4 +1,5 @@
		Adrien F. Vincent <vincent.adrien@gmail.com>
		<<<<<<< 3d31c694aa6b77b9d8b7b1897f3a67f5be1a54ea

support for updating axis ticks for categorical data #6889

support for updating axis ticks for categorical data #6889

Conversation

story645 commented Aug 3, 2016

story645 Aug 3, 2016

Choose a reason for hiding this comment

story645 commented Aug 3, 2016 • edited Loading

Kojoley commented Aug 3, 2016

tacaswell commented Aug 4, 2016

story645 commented Aug 4, 2016 • edited Loading

tacaswell commented Aug 4, 2016

story645 commented Aug 4, 2016

Kojoley commented Aug 4, 2016

story645 commented Aug 4, 2016

Kojoley commented Aug 4, 2016

story645 commented Aug 4, 2016 • edited Loading

Kojoley commented Aug 4, 2016

story645 commented Aug 4, 2016

tacaswell commented Aug 21, 2016

tacaswell Aug 21, 2016

Choose a reason for hiding this comment

story645 Aug 21, 2016

Choose a reason for hiding this comment

story645 commented Aug 21, 2016

Kojoley commented Aug 21, 2016

Kojoley commented Aug 21, 2016

story645 commented Aug 23, 2016

tacaswell Aug 23, 2016

Choose a reason for hiding this comment

story645 Aug 23, 2016

Choose a reason for hiding this comment

tacaswell commented Aug 23, 2016

story645 commented Aug 23, 2016

story645 commented Aug 23, 2016

Kojoley commented Aug 24, 2016

story645 commented Aug 3, 2016 •

edited

Loading

story645 commented Aug 4, 2016 •

edited

Loading

story645 commented Aug 4, 2016 •

edited

Loading