search

openworm / tracker-commons

Compilation of information and code bases related to open-source trackers for C. elegans
11 stars 12 forks source link

Added perimeters to WCON spec, while simplifying some things. #146

Closed Ichoran closed 7 years ago

Ichoran commented 7 years ago
Ichoran commented 7 years ago

Looks like I clobbered the Python build with this one. Maybe it will work if I fix the schema?

Ichoran commented 7 years ago

Schema is fixed to include the new features (and old features that we talked about). @MichaelCurrie and @JimHokanson - does the updated spec look reasonable to implement?

MichaelCurrie commented 7 years ago

Looks like I clobbered the Python build with this one. Maybe it will work if I fix the schema?

@Ichoran you did not. Our CI script downloads from a URL and the URL was changed 2 weeks ago. I've fixed this now: https://github.com/openworm/tracker-commons/commit/a3ce4afb7525d9650c9f971d715a989983344759

Ichoran commented 7 years ago

@MichaelCurrie - Could you check the changes to see if I've adequately captured the decisions we've made on the spec? I tried to address the custom data thing by making clearer rules about what can go in and allowing more flexibility in readers interpreting them. For instance, I dropped the ["salmon", "salmon", "cod", "cod", "cod"] thing from the core specification because data duplication can have drastic unexpected consequences in some situations (e.g. copying a huge block of text verbatim tens of thousands of times). So I don't think we can just state that it should happen.

MichaelCurrie commented 7 years ago

I think I will still implement ["salmon", "salmon", "cod", "cod", "cod"] because it will be useful. But perhaps you're right that we have no business requiring custom data to behave in one way or another.

Ichoran commented 7 years ago

@MichaelCurrie - I will implement it also; it's listed as the first option in what readers are encouraged do (or provide as an option) if they can't simply join the two lists.

Ichoran commented 7 years ago

@MichaelCurrie - There are a lot of errors now in the manually-created strings in tests.py. Do you want me to fix them, or would you rather as a way to catch places where you might need to change the parsing code also?

MichaelCurrie commented 7 years ago

@Ichoran I'm planning a mass fix of all these issues (I'm capturing them under milestone Python WCON 1.2.0), so please don't worry and let me fix them, I hope to get to them all soon and then we can accept this PR.

Ichoran commented 7 years ago

@MichaelCurrie - Okay, sounds good.

MichaelCurrie commented 7 years ago

This is breaking Python but let's merge for now.