search

airr-community / airr-standards

AIRR Community Data Standards
https://docs.airr-community.org
Creative Commons Attribution 4.0 International
35 stars 23 forks source link

RepertoireSet back to RepertoireGroup? #631

Closed schristley closed 2 years ago

schristley commented 2 years ago

@javh You snuck that in there! I'm okay with the change except I have a lot of files and code that's using it, so I have a bias to keep.

But I'll also offer a math argument in that a time series is actually a sequence and not a set... What argument would convince to change back? ;-)

javh commented 2 years ago

Yeah, it was in #629 - a quick topic from the last call. It's experimental, so I don't mind changing it back. It may change in v2.0 though. @scharch?

scharch commented 2 years ago

Uh @javh it was your idea to change the name for consistency with other field names. I don't have a strong opinion either way. Changes for v2 will be around giving it actual functionality, instead of just being a list. Not sure that impacts the name, either.

javh commented 2 years ago

@scharch, Yeah, it was my idea to match GermlineSet and GenotypeSet. I also notice the Manifest proposal has DataSets (which points to files, but DataFile points to objects).

(Just checking with you because you're the owner of RepertoireGroup.)

javh commented 2 years ago

Changes for v2 will be around giving it actual functionality, instead of just being a list. Not sure that impacts the name, either.

One other thought... This is kind of an argument for leaving it as RepertoireGroup for now. If we know that people's implementations are going to have to change for v2.0, we might as well make @schristley only rework his code once.

scharch commented 2 years ago

Sounds good, let's leave it as RepertoireGroup for now

schristley commented 2 years ago

@javh Ok, I ran VDJServer's repcalc using v1.4 master for some gene usage/combo and mutation reports which uses germline, rearrangements, repertoires and repertoire groups, and everything ran fine, so the python library at least looks ready to me.

I'm not getting the deprecated warn messages when repcalc uses load_repertoire, but I do get it if I type commands into the python interpreter, not sure why.

javh commented 2 years ago

Sweet! Thanks, @schristley.

Regarding the deprecation warning, I believe the default commandline behavior is to ignore DeprecationWarning and you should be able to change it via the PYTHONWARNINGS env variable: https://docs.python.org/3/library/warnings.html#describing-warning-filters