8000 Fix font weight validation by timhoffm · Pull Request #15271 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Comments

Fix font weight validation#15271

Merged
QuLogic merged 1 commit intomatplotlib:masterfrom
timhoffm:validate-int-like-str
Sep 16, 2019
Merged

Fix font weight validation#15271
QuLogic merged 1 commit intomatplotlib:masterfrom
timhoffm:validate-int-like-str

Conversation

@timhoffm
Copy link
Member
@timhoffm timhoffm commented Sep 15, 2019

PR Summary

Fixes #15240. Solution taken from #15240 (comment).

@timhoffm timhoffm added topic: rcparams Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labels Sep 15, 2019
@timhoffm timhoffm added this to the v3.2.0 milestone Sep 15, 2019
dstansby
dstansby requested changes Sep 15, 2019
View reviewed changes
Copy link
Member
@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

This should definitely get a simple smoke test

@timhoffm timhoffm force-pushed the validate-int-like-str branch from 29b6780 to 56c53a1 Compare September 16, 2019 19:26
@timhoffm timhoffm force-pushed the validate-int-like-str branch from 56c53a1 to 337c0b9 Compare September 16, 2019 19:27
@timhoffm timhoffm force-pushed the validate-int-like-str branch from 337c0b9 to 18cecf7 Compare September 16, 2019 19:37
@timhoffm
Copy link
Member Author
timhoffm commented Sep 16, 2019 •
edited
Loading

I actually changed the implementation to only accept a defined list of strings and trying to convert everything else to int. This allows the validator to fail early on invalid strings like "noweight". - Previously, the code using the weight would have failed on that.

The implementation uses a fixed list of strings. That is not too nice, because it duplicates font_manager.weight_dict. OTOH these won't rarely ever be changed and it prevents some circular import hazzle between font_manager and rcsetup. Also, it's precedenced by validate_fontsize() right above.

ping @dstansby Tests added.

@QuLogic QuLogic modified the milestones: v3.2.0, v3.1.2 Sep 16, 2019
@QuLogic QuLogic merged commit a813b92 into matplotlib:master Sep 16, 2019
@lumberbot-app
Copy link
lumberbot-app bot commented Sep 16, 2019

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout v3.1.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 a813b92e3947a42c410213cf846b5c1788d061b0
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #15271: Fix font weight validation'
  1. Push to a named branch :
git push YOURFORK v3.1.x:auto-backport-of-pr-15271-on-v3.1.x
  1. Create a PR against branch v3.1.x, I would have named this PR:

"Backport PR #15271 on branch v3.1.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Sep 16, 2019
Backport PR matplotlib#15271: Fix font weight validation
cf86846
@meeseeksmachine meeseeksmachine mentioned this pull request Sep 16, 2019
Merged
timhoffm pushed a commit to timhoffm/matplotlib that referenced this pull request Sep 16, 2019
@QuLogic @timhoffm
Backport PR matplotlib#15271: Fix font weight validation
67b4ee2
This was referenced Sep 16, 2019
Closed
Merged
@timhoffm timhoffm deleted the validate-int-like-str branch September 16, 2019 22:17
dstansby added a commit that referenced this pull request Sep 17, 2019
@dstansby
Merge pull request #15278 from timhoffm/auto-backport-of-pr-15271-on-…
934ba3a 
…v3.1.x

Backport PR #15271 on branch v3.1.x (Fix font weight validation)
dstansby added a commit that referenced this pull request Sep 17, 2019
@dstansby
Merge pull request #15276 from meeseeksmachine/auto-backport-of-pr-15…
01e32d0 
…271-on-v3.2.x

Backport PR #15271 on branch v3.2.x (Fix font weight validation)
QuLogic
QuLogic reviewed Sep 18, 2019
View reviewed changes
lib/matplotlib/rcsetup.py
try:
return int(s)
except (ValueError, TypeError):
raise ValueError(f'{s} is not a valid font weight. %s')
Copy link
Member
@QuLogic QuLogic Sep 18, 2019

Choose a reason for hiding this comment

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

Err, hold one, were both these s's in here?

@timhoffm timhoffm mentioned this pull request Sep 18, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@QuLogic QuLogic QuLogic approved these changes

@dstansby dstansby dstansby approved these changes

Assignees

No one assigned

Labels

Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. topic: rcparams

Projects

None yet

Milestone

v3.1.2

Development

Successfully merging this pull request may close these issues.

Can't specify integer font.weight in custom style sheet any more

3 participants

@timhoffm @QuLogic @dstansby
0