Skip to content

check freetype version also specifies what is wrong #6403

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

Closed
wants to merge 10 commits into from

Conversation

prhbrt
Copy link

@prhbrt prhbrt commented May 11, 2016

The simpler and better guided it is to setup a testing-environment for (new) developers, the more fun it is for them to develop something.

Herbert Kruitbosch added 3 commits May 11, 2016 14:23
…ytestring (not writing it to file), which can be practical in combination with the base64-package. Also addded a {figure,pyplot}.subplots_iterator which returns an iterator over subplots, which can be practical in for example ipynb's, where one wants to make many graphs and likes them to be 3 (or 5 or whatever) on a line.
"Expect many image comparison failures below. "
"Expected {0} != found {1}".format(
ft2font.__freetype_version__,
LOCAL_FREETYPE_VERSION
Copy link
Member

Choose a reason for hiding this comment

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

This message might be a little misleading, I would think, because this isn't the full condition that needs to be satisfied. @mdboom, I will defer to you on this.

Copy link
Author

Choose a reason for hiding this comment

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

Although you are right, I've actually set local_freetype=True in setup.cfg, then ran setup.py clean and setup.py install, afterwards this error message still occurred.

It makes me wonder if ft2font.__freetype_build_type__ == 'local' can actually happen.

Copy link
Member

Choose a reason for hiding this comment

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

It does work Travis uses it all the time Please check the Travis logs for instance here

https://travis-ci.org/matplotlib/matplotlib/jobs/129092969

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if setup.py clean is sufficient to remove everything that needs to be, I tend to use git clean -xfd to reset things 😉.

There is also the MPLLOCALFREETYPE env which if it is set to anything true will use the local freetype (this is what I do).

Copy link
Author

Choose a reason for hiding this comment

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

@tacaswell You might be right. What also seemed to have worked is removing the ubuntu, the virtualenv, and re-executing setup.py install :)

@tacaswell tacaswell added this to the 2.1 (next point release) milestone May 17, 2016
@prhbrt
Copy link
Author

prhbrt commented Jul 19, 2016

Removed stuff from different features from this branch, I hope this will pass the coverage tests, as nothing interesting really changed, except that people get better error messages.

@QuLogic
Copy link
Member

QuLogic commented Jul 19, 2016

Please rebase and squash out the reverted changes.

@prhbrt
Copy link
Author

prhbrt commented Jul 20, 2016

@QuLogic Thank you for the advice, but I'm not going to do that, because I don't like wasting my time. If someone wishes, he or she can do so. Otherwise, this PR can be rejected.

@matthew-brett
Copy link
Contributor

@prinsherbert - to squash and rebase is not wasting your time, it is cleaning up the history to make it clearer to other developers what has been changed.

@prhbrt
Copy link
Author

prhbrt commented Jul 20, 2016

@matthew-brett obviously, and I get why its preferable, yet I do not have the time to find out how this works and execute it, and hence won't do it.

@matthew-brett
Copy link
Contributor

I see - but in that case I think the most pleasant and efficient way to proceed is to say "I am sorry, I am not experienced with git, would one of you mind doing the squash and rebase for me?".

@prhbrt
Copy link
Author

prhbrt commented Jul 20, 2016

That is a nice sugar-coating, I admit :) Sending me the commands is also OK, nevertheless I remember trying to do this cherry-pick/rebase related git-stuff before, and wasn't able to do what I wanted and ended up creating a new clone with a new branch, copying the changes from the previous clone, and then creating one commit: tedious you must admit. Especially when github already is aware of the dirty commits, I'd have to create a new branch and a new PR, the boilerplate code management never ends, and probably I would get comments on the way I've dealt with the second branch I made. I just decided to never touch or change commits in order to lead a happier life.

@matthew-brett
Copy link
Contributor

Sure - but not just sugar coating right? Your options are to either make it my fault (by saying you don't want to waste your time, implying the suggestion is silly) or your fault (by saying you don't know how to do it). I am completely confident that you'll get lots of help with the second, but it's easy for people to get tired and annoyed by the first.

Actually, now I look at the history, it's not trivial at all to do a rebase, I wonder whether it is worth it. @QuLogic - did you have a look already - what do you think? Probably does need an expert to do the history work.

@prhbrt
Copy link
Author

prhbrt commented Jul 20, 2016

@matthew-brett I did not imply that any suggestion is silly; if I would, I would have provided arguments.

@matthew-brett
Copy link
Contributor

I'm sorry - I am sure it is a language thing - "I don't want to waste my time" in English does usually imply that the suggestion is at fault. Like the English themselves, the language can be tricky and subtle at times.

@prhbrt
Copy link
Author

prhbrt commented Jul 20, 2016

The suggestion is fair, and I was obtuse; I am sorry for that. Meanwhile, I guess, I could have learned more on rebasing :). I honestly do think it is a waste of time to do so given the minor change in code - but I also think it's not up to me to decide on how PR's are done here.

I came across the error message while running tests, and I always detest it when error messages say something in some area went wrong, but do not specify any details (like a file not found, without stating which file). If this happens, I almost always end up adding print-statements to the code, even if it's site-package based. I thought, why not add my print-statements to the error message. I was silly though, and put it between the changes of a different feature, and committed that two months ago, never deleted the e-mail from github from my inbox, and decided to go and clean this up yesterday. The reason: I think lack of contextual information in error messages causes a lot of time being wasted by debugging; especially developers trying to solve the same problem on different machines. If I have the problem why wouldn't others also have it?

Now, since this discussion is still going on, I'm wondering how much effort it takes if you want to make a minor change (and imho this still is a minor change) which can help many others. I just wanted to contribute a little bit, but I don't have the time to adhere to all the requirements matplotlib requires to accept a PR. Even though I went through the effort of creating a matplotlib development environment on my laptop, it is unlikely that I will create new PR's for matplotlib whenever I see something that could be done better. I often use matplotlib, and also browse through the code often; for example when I need to find out how something works. I often have ideas on how to improve the interface in clarify or usability, which I also discuss which others IRL to fine-tune them.

This is the frustration I admit was there in my comment about wasting time, and I am sorry to have bothered others with it.

@matthew-brett
Copy link
Contributor

It's OK - frustration is useful sometimes.

It's always a tension for projects, between making it easy to contribute, and making sure the resulting code (and here, history) is in good shape. Some projects - like sympy - don't ever rebase, partly in order to make it easier to contribute.

I wonder whether matplotlib could use a policy like offering to do a rebase for new contributors.

@prhbrt prhbrt closed this Jul 20, 2016
@prhbrt prhbrt deleted the freetype-version-check branch July 20, 2016 13:08
@prhbrt
Copy link
Author

prhbrt commented Jul 20, 2016

I was hoping I could sneak-fix the problems by cloning the original master, adding the changes and pushing them as one commit. This closed this PR, but a similar one now is here: #6803

@QuLogic QuLogic modified the milestones: unassigned, 2.1 (next point release) Jul 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants