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

Support keyword arguments for functions. #8

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

pat
Copy link

@pat pat commented Mar 10, 2021

Another Ruby 3 related patch: now that keyword arguments are no longer implicitly translated into hashes, we need to explicitly handle them as a separate argument type.

And I must be clear: I don't have a deep understanding of this gem, so I feel like the specs and code are quite possibly incomplete. Hopefully they're at least useful as a starting point. 😅

I presume it’s something that was refactored out of dry-core?
@pat pat requested a review from solnic as a code owner March 10, 2021 02:19
pat added 3 commits March 10, 2021 14:01
Now that keyword arguments are no longer implicitly translated into hashes, we need to explicitly handle them as a separate argument type.

This commit may not cover all cases (either from a code or spec perspective), but hopefully it’s at least a useful start.
This means the kwargs are always present in the AST, thus updating all the expected responses.
I’m presuming this was committed accidentally 😅
@pat
Copy link
Author

pat commented Mar 10, 2021

… and now that I removed the :focus that avoided the full suite from running, I can see that the build is not happy at all with pre-3.0 Rubies. And I'm feeling well and truly out of my depth.

I might set this aside for now, rather than muddling my way through the failing tests. Hopefully what's here is useful, but for now I think I'm going to change my functions to avoid keyword arguments.

@solnic
Copy link
Member

solnic commented Mar 10, 2021

Thanks. It's best to hold off with this. I think it would be good to revisit the implementation of this gem in general, given how many things changed in Ruby since transproc was born (which is now dry-transformer).

@nonnenmacher
Copy link

got exactly the same problem when moving a running project from 2.7.x to 3.1.

My solution was vastly simple (naive).

i.e adding a kwargs build from given *args Array in functions.rb
along the lines


def initialize(fn, options = {})
        @fn = fn
        @args = options.fetch(:args, [])
        @kwargs = {}
        if fn.parameters.any? { |(param_type, _)| param_type == :key}
          if args.last.is_a? Hash
            @kwargs = @args.pop
          end
        end
        @name = options.fetch(:name, fn)
      end

      # Call the wrapped proc
      #
      # @param [Object] value The input value
      #
      # @alias []
      #
      # @api public
      def call(*value)
        fn.call(*value, *args, **kwargs)
      end

worked for a simple composition of Dry::Transformer::Pipe

But then discovered this more extensive and nicer version.

Is there is plans to revisit this and perhaps drop support for pre 2.7 on a recent version ?

This gem is nice and perhaps need a bit of love ;-)

@rfb
Copy link

rfb commented Jan 17, 2023

Ah, I'm having the same issue using prefix: kw arg with unwrap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants