Why not allow None in an appstruct serialized by Number?

[tisdall]

relates to part of #186

A commit was made a while ago that removed the ability to pass None as an appstruct value to a Number type ( commit 513d860 ). The commit says it's reverting a change related to Strings but then made this change to Number. It seems like an odd change and is removing useful functionality.

Basically, it comes down to the fact colander.null is not equal to None. This makes sense in the cases where None is a valid serialization. From the docs:

Note that colander.null has no relationship to the built-in Python None value. colander.null is used instead of None because None is a potentially valid value for some serializations and deserializations, and using it as a sentinel would prevent None from being used in this way.

However, in the cases where None is not a valid value, why not treat it as equal to colander.null for simplicity? The very common use case is when using an SQL DB for storing of values. An integer column that's nullable can store both integers and None.

Incidentally, where is the case where None is a valid value?

[lei12]

+1
Any improvement on this issue lately ?

[tisdall]

No, and don't expect any. :(

[lei12]

why not ?

[tisdall]

Well, this issue is nearly a year old and there's other related issues that are much older (ex. #140)...

[stevepiercy]

Aside from availability of volunteers to do the work (See #208 (comment)), all changes should include tests, documentation, and other standard release requirements according to the Pylons Project guidelines.

Is there a PR somewhere related to this issue?

[tisdall]

Before a PR is created, answers need to be made to questions in the original issue, no?

[stevepiercy]

Nope. A PR can help by giving maintainers something concrete to review, discuss, and take further action on. It does not need to be perfect. Plus it shows that people who are not the maintainers will take initiative to do the work and contribute to the project.

[tisdall]

I've already put forward PR's that have gotten no response. I think I have over 10 across the different Pylons projects. If a discussion isn't being made on the open issues that exist, I don't see why anyone should bother with the effort of making patches, documentation, and tests. Someone needs to review the issues at hand and if they see merit they could ask for a PR or close them.

As for this specific issue... I don't know why the change was made. I could create a PR that reverts the change, make docs, make tests, and then after all that be told "oh.. we did that because of X" and I just wasted a bunch of time.

[stevepiercy]

@tisdall Sorry to go meta here and slightly off-topic...

I agree with you about the lack of response. At the same time, there are too few maintainers for colander and other Pylons Project projects who are qualified to take action.

I think some of the Pylons Project projects are hitting the same barrier of other FOSS projects: too little time available by the maintainers to do the needed maintenance work, and no new qualified volunteers asking to become maintainers. We need fresh meat!

I don't know whether money would help mitigate the situation. I'm compiling data points across all the Pylons Project projects to assess which might benefit from grant funding. Ultimately I plan to apply for grants (e.g., PSF, MOSS, etc.) to compensate someone to do the work. Money is not a primary motivator when it comes to FOSS, but it helps improve its priority above "I've got better things to do than work more hours at a computer". I personally think that more maintainers across the projects would be a good thing™, whether that involves remuneration.

[tisdall]

^_^ I think we already went off topic.

I already had this sort of discussion on IRC. You'd think a history of good PR's would be enough to accept someone as a maintainer, but apparently it's not. I'm not involved with any projects utilizing Pyramid so I've since lost interest in being a maintainer.

[digitalresistor]

24f648d was the original commit that added the feature described in #59, however the revert that happened in 513d860 claims to only target the change made in #73, but that clearly was not the case. There is also no CHANGES note made about the change to the way that Numbers are deserialized which is confusing because if you look at the CHANGES doc and find the comment about #59, you would expect that you could pass None to serialize.

Just ran into this while working with @stevepiercy and we just sub-class the Integer and updated serialize, but I do think it is a valid concern and one we should address.

All things old are new again.

@mmerickel if I made a PR that fixed this for Number would you be opposed to the change?

[mmerickel]

It's fine with me.