Skip to content

Conversation

@amcgregor
Copy link
Contributor

Please consider this patch. It:

  • Removes special cases. The former verbose_name, help_text, and custom_data special cases have been removed and are fully covered/supported by this change. Those test cases didn't even need to change, and it's likely advisable to continue to document de-facto standard labels for new users.
  • Saves space. Only store metadata you actually care about, i.e. saves three None associations on every field if you, like me, don't use the aforementioned metadata in every project. As __slots__ is not in use, those would be stored in the self.__dict__ mapping. (Which is what this patch updates flexibly instead.)
  • Removes unnecessary indirection. Additionally, every access to that metadata requires at least an extra attribute lookup, and will almost always have an additional attribute lookup or array dereferencing beyond that, to support storage of multiple metadata values.
  • Type less, get more done. Use of custom_data as a dictionary adds 20 extra characters to every declaration, or more if using the JSON-like dictionary literal with more than two key/value pairs.
  • Eliminates glue. With custom_data being what it is, a fixed name, use with most widget libraries will continue to require adaption glue. Is the value of custom_data a dictionary, or instance of a custom class? More glue. Arbitrary field metadata allows the application author to adapt MongoEngine to the widget library more easily, through plain declaration of the attributes required by the library. As the author of a web framework that integrates MongoEngine, and a template/widget framework, glue is a concern for me.
  • Has documentation. Apparently custom_data wasn't documented.

Thank you for your consideration.

Review on Reviewable

Includes ability to detect and report conflicts.
Removes `verbose_name`, `help_text`, and `custom_data`.  All three are
covered by the one metadata assignment and will continue working as
expected.
@amcgregor
Copy link
Contributor Author

Tests fail on Mac due to missing zlib when installing Pillow? Quite strange, but usage seems OK.

@amcgregor
Copy link
Contributor Author

This is becoming a growing issue; I strongly prefer not to run tailored forks in production environments. ;)

@amcgregor
Copy link
Contributor Author

@seglberg / @thedrow Ping? Any hope?

@thedrow thedrow merged commit c0e7f34 into MongoEngine:master Nov 23, 2015
@amcgregor amcgregor deleted the feature/arbitrary-metadata branch November 23, 2015 16:00
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.

1 participant