Use Pydantic BaseModel instead of dataclass

[tlambert03]

This PR ports the autogen code over to use the pydantic BaseModel instead of the pydantic dataclass. The behavior is essentially the same (the two things are extremely similar in design and function), but this makes a number of things more straightforward, makes inheritance much easier (no funny business with non-default args following default args), and I hope this will make it easier to pull some of the stuff out of the autogen code and into some "real" python that we can lint/check etc...

@jmuhlich, I know you're busy, but if you have a moment to let me know if you're terribly against this idea, that'd be great. Otherwise, I'll probably push forward with it.

(edit: I've taken the opportunity to clean up a number of other things here as well, fixes #66, and improves typing around ids, so this PR has unfortunately become a bit bloated)

[tlambert03]

actually @JacksonMaxfield, if you have a moment to run your tests against this branch, that'd be helpful too. It shouldn't change anything externally... but would be nice to know that it doesn't affect your stuff before merging to master

[evamaxfield]

Hey @tlambert03 seems like there is a single change from current as a result of this!

Seems that the OME.images[i].pixels.dimension_order is now a string rather than something else? To be honest I don't know what type it was before but this is my code currently:

# Create dimension order by getting the current scene's dimension order
# and reversing it because OME store order vs use order is :shrug:
dims = [d for d in scene_meta.pixels.dimension_order.value[::-1]]

Simply dropping the .value at the end is enough to make my tests pass again!

# Create dimension order by getting the current scene's dimension order
# and reversing it because OME store order vs use order is :shrug:
dims = [d for d in scene_meta.pixels.dimension_order[::-1]]

[tlambert03]

Thanks! That's very helpful. I think this has to do with the way the pedantic constrained string classes had a value attribute (instead of just acting like a string). This behavior actually seems more desirable! I'll check to see if it affects anything else

[evamaxfield]

Agree that this behavior seems more valuable! So no worries for me there!

[evamaxfield]

This may break some stuff for @toloudis. (re: your temporary OmeTiffWriter / metadata cleaner, I would just strict version pin ome-types until 4.x OmeTiffWriter is released)

[tlambert03]

I’m also happy to try to make it backwards compatible here too if you want to point me to anything it breaks on your end @toloudis

[toloudis]

I'll have a chance to pull this branch and try it out tomorrow! Thanks for the heads-up. I think the dimension_order thing looks like an improvement.

[tlambert03]

actually. @JacksonMaxfield and @toloudis, I haven't really seen any of the code where you use ome-types. If you can point me to the most recent branches that I could look at, I'd be curious, and I can add some more tests here to make sure your needs are being tested locally as well

[evamaxfield]

All of the 4.x code is on main for aicsimageio: https://github.com/AllenCellModeling/aicsimageio/tree/main

I don't have access to @toloudis code anymore unfortunately

[evamaxfield]

Specifically here is the OmeTiffReader: https://github.com/AllenCellModeling/aicsimageio/blob/main/aicsimageio/readers/ome_tiff_reader.py

[tlambert03]

thanks!

[tlambert03]

OK, so... I could make this compatible with your current tests by undoing just one line in this PR: setting use_enum_values back to False in the pydantic Model Config.

Do you like all of the enum values to be raw Enum instances? or just strings? I'm torn; I can see +/- either way... sometimes it's nice to have the raw enum object to see all available options.

A third option is to use an Enum subclass that provides a __str__ method, so that you could do str(scene_meta.pixels.dimension_order)[::-1] (as well as scene_meta.pixels.dimension_order.value)

[evamaxfield]

Yea on giving it some more thought I think having the raw Enum objects returned is probably the way to go? Idk I am on the fence too but I think I side in that direction. Simply because I like the explicitness of it all. Plus allowing the other enum options like name is cool too.

[tlambert03]

Idk I am on the fence too but I think I side in that direction.

great. I'm leaning that way too. It's easy to get the string from the enum, but hard to go the other way. will stick with that. That should mean this PR won't affect your stuff at all

[toloudis]

All of the 4.x code is on main for aicsimageio: https://github.com/AllenCellModeling/aicsimageio/tree/main

I don't have access to @toloudis code anymore unfortunately

https://github.com/allen-cell-animated/cellbrowser-tools/ has been public for a while (arguably it shouldn't be yet, but oh well!)
This PR is where I stopped using aicsimageio 3.x OME metadata handling and made my stuff pass thru ome-types:
https://github.com/allen-cell-animated/cellbrowser-tools/pull/5/files

[tlambert03]

Thanks both, I've reverted the enum change back to the original behavior. so i think at this point it shouldn't change your tests. will merge tomorrow. feel free to keep suggesting changes to #77 if you'd like to ensure more things are tested on my end against aicsimageio

[tlambert03]

I just released 0.2.4 without this change. Will merge now and when it releases it will be 0.3.0