Skip to content
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

Changed default frombuffer raw decoder args #1730

Merged
merged 2 commits into from Oct 2, 2019

Conversation

radarhere
Copy link
Member

The comment suggests changing the default values post 1.1.6, so that they are in line with the warning displayed to the user.

Sounds reasonable.

@wiredfool
Copy link
Member

I know the comment says that this will change post 1.16.

Does this improve the functionality? Is this likely to break anyone in the wild?

@radarhere
Copy link
Member Author

My theory is that anyone who has used this previously would have started explicitly listing their arguments, rather than continue to receive the warning. The only improvement I intended was to remove the warning, and allow many users to skip straight to the recommended defaults.

If you would like to close this however, in line with the policy of minimal changes, then you should do so.

@aclark4life
Copy link
Member

I don't know that I want to close it, as much as I'd like to see it fully explored and either ruled out or in. We do want to make minimal changes in some cases; in other cases, we want fix long-standing annoyances not to mention add new features.

@radarhere
Copy link
Member Author

Just to throw out another idea - we could change the default behaviour and update the warning to report that change.

@hugovk
Copy link
Member

hugovk commented Feb 22, 2019

The warning has been there since PIL 1.1.6 (December 3, 2006). The frombuffer function was added in 1.1.4 (May 10, 2003).

Looking up the original reason for the warning:

  • The default arguments for "frombuffer" doesn't match "fromstring"
    and the documentation; this is a bug, and will most likely be fixed
    in a future version. In this release, PIL prints a warning message
    instead. To silence the warning, change any calls of the form
    "frombuffer(mode, size, data)" to

    frombuffer(mode, size, data, "raw", mode, 0, 1)
    

https://bitbucket.org/effbot/pil-2009-raclette/src/cd403356263f039a4a48a1111c7f5cc38686e481/CHANGES#lines-306

fromstring has since been replaced with frombytes:

    def fromstring(self, *args, **kw):
        raise NotImplementedError("fromstring() has been removed. "
                                  "Please call frombytes() instead.")

Is it still the case that the default args for frombuffer don't match frombytes and the docs?

@radarhere
Copy link
Member Author

The docs for writing your own file decoder state that

If omitted, the orientation defaults to 1

@hugovk
Copy link
Member

hugovk commented Sep 21, 2019

How about we update the warning to concretely say:

-the frombuffer defaults may change in a future release
+the frombuffer defaults will change in Pillow X

And then do the change in version X? Version 7.0.0 isn't a lot of notice, but the warning's been there for ages.

@hugovk
Copy link
Member

hugovk commented Sep 24, 2019

PR to change the wording to "the frombuffer defaults will change in Pillow 7.0.0": #4086.

@hugovk hugovk added this to the 7.0.0 milestone Sep 24, 2019
Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Good to merge once today's release is out.

@radarhere radarhere merged commit f92f429 into python-pillow:master Oct 2, 2019
@radarhere radarhere deleted the frombuffer_args branch October 2, 2019 09:26
@radarhere radarhere moved this from New Issues to Done in Pillow Apr 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Pillow
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants