Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

support for updating axis ticks for categorical data#6889

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
tacaswell merged 1 commit intomatplotlib:masterfromstory645:category
Aug 25, 2016

Conversation

story645
Copy link
Member

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()

unknown

Which as a sidenote, it didn't work before because apparently FixedLocatorconverts 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

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

Choose a reason for hiding this comment

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

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

@story645
Copy link
MemberAuthor

story645 commentedAug 3, 2016
edited
Loading

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
Copy link
Member

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
Copy link
Member

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

@tacaswelltacaswell added this to the2.1 (next point release) milestoneAug 4, 2016
@story645
Copy link
MemberAuthor

story645 commentedAug 4, 2016
edited
Loading

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
Copy link
Member

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
Copy link
MemberAuthor

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
Copy link
Member

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 outthis 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 useraise KnownFailureTest (I should separate PR forknownfailed function from#6730) and it will work with pytest without changes.

@story645
Copy link
MemberAuthor

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
Copy link
Member

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
Copy link
MemberAuthor

story645 commentedAug 4, 2016
edited
Loading

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
Copy link
Member

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:

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
Copy link
MemberAuthor

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

@story645story645 mentioned this pull requestAug 10, 2016
5 tasks
@story645story645force-pushed thecategory branch 2 times, most recently fromb726feb toa8eac2cCompareAugust 17, 2016 11:03
@tacaswell
Copy link
Member

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

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

[tox]
envlist = py26, py27, py31, py32
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

sure.

@story645
Copy link
MemberAuthor

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

@Kojoley
Copy link
Member

No,#6920 does not supercede#6730.

@Kojoley
Copy link
Member

@story645 please remove the test fromdefault_test_modules var.

story645 reacted with thumbs up emoji

@story645story645force-pushed thecategory branch 2 times, most recently fromc7cb09a to3a8ec8eCompareAugust 23, 2016 14:54
@story645
Copy link
MemberAuthor

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

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

Choose a reason for hiding this comment

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

merge conflict flags

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

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

@tacaswell
Copy link
Member

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

@story645
Copy link
MemberAuthor

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

@story645
Copy link
MemberAuthor

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

@Kojoley
Copy link
Member

I have opened a PR#6974 for the issue

@tacaswelltacaswell merged commitee8f448 intomatplotlib:masterAug 25, 2016
@story645story645 deleted the category branchMarch 7, 2017 01:30
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
v2.1
Development

Successfully merging this pull request may close these issues.

3 participants
@story645@Kojoley@tacaswell

[8]ページ先頭

©2009-2025 Movatter.jp