raml 1.0 Objecttype

[tardyp]

This is copy of https://github.com/econchick/ramlfications/pull/3 which already has a fair amount of discussion in it. I am not so sure where we want to continue discussion, probably it is better there, as we'll get context.

Since yesterday I had little time to work on it, and just added a version of OrderedDict with more concise repr.

---

Remaining things todo in this PR:

• [x] String should inherit from Scalar Type

Remaining things todo in another PR:

• [ ] includes and libraries support
• [ ] final decision on wether we separate pure type declaration from parsing and data validation

[codecov-io]

Current coverage is 96.87%

Branch #80 has no coverage reports uploaded yet.

Powered by Codecov. Updated on successful CI builds.

[tardyp]

added support for basic raml fragments (to help for the unit tests) added support for data validation, as I think the validation part is too important to patch it later

don't know if you heard of reviewable, but it looks like an alternative to github for the review, especially for this PR, which will probably get big. There is a possibility to easily review changes between PR versions https://reviewable.io/reviews/spotify/ramlfications/80

[econchick]

Hey @tardyp -

I'd still like to separate out the functions you are using to parse the type objects from the object definition itself. I would put the BaseType, ObjectType etc definitions in ramlfications/raml.py since that module is meant for defining objects in the "first level" of a RAML file, e.g. traits, resource types, resource nodes. I'm open to doing something like ramlfications/raml/ with raml.py and types.py (and then annotations.py etc). The only blurry line is that the built-in scalar types, e.g. string/number/integer types are very similar to ramlfications/parameters.py with some similar/same attributes. I'd then put the parse_properties, create_property and create_type in a separate module within ramlfications/parser/, or even within ramlfications/parser/parameters.py

I really do not like how api.types would return a dictionary. It's not congruent with the rest of the package's API, where it just returns a list of the objects.

I'm hesitant about the logic in ramlfications/parser/__init__.py & ramlfications/loader.py. Is ever a time where a RAML file would just be a DataType and not a part of a complete API definition? I had imagined that a "main" RAML file would !include a DataType RAML, rather than just be by itself. And with that, there wouldn't need to be any special if/else logic when parsing, it would just be all in the complete raw dictionary. Also, what happens with this logic when a data type is defined inline within one main RAML file?

With that, I would add tests for using the !include functionality as well as when types are defined inline.

There has also been this open-ended question of when something is not defined, is it None, or a default value, or not a Python None but more of like a Nothing object. For instance, if an Object data type is defined, and no minimum properties are defined, is it None? or should it just be 0? Or should we reflect that it was truly not defined with a Nothing-type of object? I've always defaulted to the first one (unless the RAML spec specifically defines a default value, e.g. query/uri/form parameters will default to "string" types). With your definition, you have a default values defined, which is not congruent with the current API, and something I would be very hesitant to add (generally for lack of reliability; for instance, MAXSIZE would be different across different python implementations and systems). I've been thinking of implementing a Nothing object similar to how attrs handles it. In which case, I think it's best to just return None if nothing is defined for right now, as that is how the API behaves elsewhere.

[tardyp]

re-separating object definitions from parsing: I understand your wish for this, for me the code is easier to read like this rather than having to go back and forth from one file to the other.

re-factorization with parameter: this is something I haven't looked at yet, indeed this is something we should take care of, and also we should factorize with the annotation types you are working on as well.

re-dictionary: api.types is now a list.

about using ramlfication to directly import a DataType file, I do plan to use it as a feature in order to implement data-validation. I think also Library makes sense to be loaded independently from an actual API using it. For me this makes a lot of sense to share data types library between projects.

I hadn't look at !include yet, and Libraries, and wanted to do so in another PR.

None vs Nothing makes a lot of sense. will do.

[tardyp]

see PR description for remaining TODOs

[tardyp]

pushed a new version addressing some of the comments. rebased over https://github.com/spotify/ramlfications/pull/83 so I had to squash the history in order to ease up rebasing

[tardyp]

Added support for scalar, int and bool. I think we can merge this like this, and continue the work in further PRs (including split bw model and logic if you really think it is more suitable)

[tardyp]

@econchick let me know if you need more work on that PR, else, fill free to shipit!

[econchick]

@tardyp Ok I think you need to rebase from v0.2.0 branch now since I merged #83

[tardyp]

@econchick as, this PR is already on top of #83, there is no need to rebase

[econchick]

@tardyp unfortunately github isn't smart enough, and still shows that your first commit isn't merged, but will after a rebase.

[tardyp]

here is the rebase!

[econchick]

:heart: thank you @tardyp !