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

Options for valid method #9

Closed
frozzare opened this issue May 30, 2019 · 6 comments
Closed

Options for valid method #9

frozzare opened this issue May 30, 2019 · 6 comments
Projects

Comments

@frozzare
Copy link
Member

frozzare commented May 30, 2019

We should add a options to valid method to disable coordination number as discussed in personnummer/python#5

Something like this:

personnummer.valid(number, { coordination: false })

We should also update the package specification readme.md

ping @Johannestegner

@Johannestegner
Copy link
Member

Yes, this is probably a good idea. Should the coordination number be true or false by default though?

I feel it will be a breaking change if we set it to not be included as default, as is have been "active"by default since implementation.

@frozzare
Copy link
Member Author

We shouldn't break anything so coordination number should be true by default. It's also a valid personnummer :)

@frozzare
Copy link
Member Author

frozzare commented Oct 4, 2019

After some discussion on slack, we'll add this v3 of the spec. #15

Function(string|number, object)

Example

Valid(ssn, { coordinationNumber: false })

ping @Johannestegner

@frozzare frozzare mentioned this issue Oct 4, 2019
@frozzare frozzare added this to In Progress in Board Oct 16, 2019
@dannyhajj
Copy link
Member

I first posted this in #15, but I'll delete the other comment and move the discussion here.

Regarding accepting an object as a second parameter.

Can this be language-specific? Or at least do not implement this in the Python package?
In Python, it's more common to have keyword arguments instead of objects. They are more explicit and can be easily documented with the proper default value.

For example, the following two functions, foo and bar, have almost the same signature.
_The difference, however, is that bar will accept and ignore any named parameter other than the defined ones. While foo will raise an error.

def foo(param1, param2=False, param3=True):
    pass

def bar(param1, **kwargs):
    param2 = kwargs.get('param2', False)
    param3 = kwargs.get('params3', True)

And both can be used like so:

foo('foobar')
foo('foobar', param3=False)
foo('foobar', param2=True)
foo('foobar', param3=False, param2=True)
bar('foobar')
bar('foobar', param3=False)
bar('foobar', param3=False, param2=True)

params = { 'param2': True, 'param3': False }
foo('foobar', **params)
bar('foobar', **params)

foo('foobar', param4=False) # Raises an error for an unexpected keyword argument
bar('foobar', param4=False) # No error, but `param4` will just be ignored.

But only foo can be used with positional parameters for param2 and param3

foo('foobar', True, False)
bar('foobar', True, False) # This raises an error

@frozzare
Copy link
Member Author

@dannyhajj

First of all I will say thank you to your great comment about how this works in python and giving us input about language-specifics issues.

We have had a discussion about this and to allow language-specific code for both options and how errors are handeld. We will up the spec document and #15 about all that we decided about what v3 should contain.

@frozzare
Copy link
Member Author

Version 3 of the spec allows to create language specific options and error handling. The aren't so much options left in version 3 either since we removed the coordination number option and replaced it with a class method. https://github.com/personnummer/meta#package-specification-v3

Board automation moved this from In Progress to Done Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Board
  
Done
Development

No branches or pull requests

3 participants