Refactor of settings.py

[Brachi]

Changed several parts of the code to reduce verbosity and make it a little more readable. No functionality changed. Added 2 TODOs to discuss functions that return a string or None, to either update the docstrings or the functions + the tests

[Brachi]

Can the docstring be updated? Not sure if changing the return type to be only string is a good idea at this stage.

[pitbulk]

I don't think is a good idea, let's keep return types (and only change that if strictly necessary)

[Brachi]

I updated the docstrings and removed the TODO comments.

[joac]

Code looks more pythonic, great job!

[pitbulk]

I implemented that toolkit from the php-saml toolkit and I willfully tried to keep the structure between the toolkits (same with the java and dotnet) in order to help me in the global maintenance. But I think this specific change that you proposed does not affect me to port future changes on the rest of the toolkits.

I will review and merge.

Thanks for contribute! (I will apply those changes also on python3-saml

[Brachi]

@pitbulk you've done a terrific job maintaining this library in many languages, this PR was just to make it more readable to those used to Python conventions, and I agree the structure should be kept as much as possible (e.g. although in Python getters and setters are not common practice, I'd leave them there).
I will open an issue for discussion first for any changes more than just style.

[pitbulk]

Thanks @Brachi , I read fast your comment and saw the"terrific" word that alarmed me. This is a spanish "false friend".

Thanks for contribute! Sure we can find a balance between portability and python code style :)

[pitbulk]

I tried to apply this change to python3-saml and the tests failed. I will need to review if we changed the behavior: https://github.com/onelogin/python3-saml/pull/24/files

[Brachi]

@pitbulk it seems tests/src/OneLogin/saml2_tests/response_test.py is failing at setup time in both master and the settings_refactor branches. If that test file is removed all other tests pass (including settings_test). Is that the failure you had?

[pitbulk]

never mind, the problem was in the tests :)