[ENH] add code and test to help create schemas

[Remi-Gau]

TODO

• [x] use attrs in all constructors:
  • may require refacrtoring and reordering of class hierarchy with a much leaner parent class
• [ ] add doc

unrelated changes

• reordering of imports using precommit
• take tests out of package fodler

[Remi-Gau]

@satra do you want me to fix the failing test as part of this PR? from the error report it seems to come from a test file that was not touched in this PR

[Remi-Gau]

@satra do you want me to fix the failing test as part of this PR? from the error report it seems to come from a test file that was not touched in this PR

well I went ahead and fixed it

[Remi-Gau]

@satra will ping you when this monster of a PR is ready for review. May even help to have a video chat about it.

I suspect that this PR will concentrate on the "create" part of the CRUD approach you mentioned here: https://github.com/ReproNim/reproschema-py/issues/3#issuecomment-854649078

[satra]

@Remi-Gau - thank you!! there are a few ways i can see this going. i can leave comments and we can iterate before we merge into master. or treat this as an initial version into master and refactor into full attrs or pydantic (may provide easier validation). for example, the latter would mean that all the set_* become assignments. or we could create a dev branch to merge this into and iterate on that branch.

[Remi-Gau]

that is an option indeed.

I will try to first figure out the best way to deal with attrs and class inheritance and maximize the amount of validation we can put in the constructors.

will ping you after that (whether it works or fails)

[Remi-Gau]

I will try to first figure out the best way to deal with attrs and class inheritance and maximize the amount of validation we can put in the constructors.

Personal note:

using attrs' __attrs_post_init__ to hook into initialization does not play well with inheritance.

The following code

import attrs
from attrs import define, field
from attrs.validators import instance_of, optional

@define
class Base:
    type: str = field(
        kw_only=True,
        default=None,
        validator=optional(instance_of(str)),
        converter=attrs.converters.default_if_none(default="unknown"),
    )
    schemaVersion: str = field(
        kw_only=True,
        default=None,
        validator=optional(instance_of(str)),
        converter=attrs.converters.default_if_none(default="1.0.0-rc4"),
    )

    def __attrs_post_init__(self):
        self.spam = "eggs"

@define
class Derived(Base):
    prefLabel: dict = field(
        kw_only=True, default={"en": "default"}, validator=[instance_of(dict)]
    )

a = Derived()

leads to

Traceback (most recent call last):
  File "/home/remi/github/reproschema-py/tmp.py", line 35, in <module>
    a = Derived()
  File "<attrs generated init __main__.Derived>", line 9, in __init__
  File "/home/remi/github/reproschema-py/tmp.py", line 25, in __attrs_post_init__
    self.spam = "eggs"
  File "/home/remi/github/reproschema-py/env/lib/python3.8/site-packages/attr/_make.py", line 1057, in __setattr__
    _obj_setattr(self, name, nval)
AttributeError: 'Derived' object has no attribute 'spam'

From the attrs doc

the moment you think that you need finer control over how your class is instantiated than what attrs offers, it’s usually best to use a classmethod factory or to apply the builder pattern.

So I guess I should figure out how those work.

[satra]

you may find pydantic to be easier to deal with than attrs.

[Remi-Gau]

Honestly I think that the library is not the issue. The issue is this piece of code I created is a flying spaghetti monster and as agile as my aging knees: I find it hard to predict when something is going to break or understand why they do when they do... So I will slowly refactor it into something that's easier for every one to understand. Also I would rather get a better feel for attrs before giving up.

[satra]

I would rather get a better feel for attrs before giving up

that's fine. we use both attrs (in pydra) and pydantic (in dandi schema). i simply meant that pydantic would likely provide an easier validation route, since it is built as a data modeling framework and has a lot of the boilerplate baked in.

[Remi-Gau]

other issue: I was raised on Matlab and OOP is not yet something I am fluent in. :-)

[Remi-Gau]

Things are starting to look better.

There is still potential for refactoring but I may wait a bit before doing this.

Want to test it on COBIDAS first and add some doc with sphinx to make sure whoever revisit this will not be thoroughly lost.

[Remi-Gau]

Note that not all parts of the schema have been covered yet.

FYI: I tend to start from the main one in those and implement the ones that are needed to run some tests to create some example found in the library of the reproschema repo itself

https://www.repronim.org/reproschema/30_schema/

[satra]

@Remi-Gau - we will be getting back to this soon. a new person has joined our group. i wanted to check in on the status.

a few questions:

• is there an intentional reason to move to sphinx from mkdocs?
• there is a README.md in each code folder. this seems quite different from python practices, where such statements would simply be in the code files. (it also makes packaging a little weirder since those files have to be explicitly included).

[Remi-Gau]

@Remi-Gau - we will be getting back to this soon. a new person has joined our group. i wanted to check in on the status.

status after playing around with attrs I indeed started switching to pydantic as I was already adding tons of type hints... but then I had to go back to do other things.

a few questions:

* is there an intentional reason to move to sphinx from mkdocs?

Not really except for the fact that if we want to deploy documentation useful for devs it will be much easier with sphinx given its autodoc features.

* there is a README.md in each code folder. this seems quite different from python practices, where such statements would simply be in the code files. (it also makes packaging a little weirder since those files have to be explicitly included).

yeah those should probably go away and be included as doc strings in the top of the relevant modules

[satra]

thanks @Remi-Gau - there are a few new kid techs on the block that may make our lives even easier. i'll see where we can take this soon.