style: remove unused regex and consolidate curator tag logic #3200
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 4 commits
August 28, 2022 23:08
Technically our Connexion API def is doing this for us, but it is best to be accurate in our code.
danieljhegeman
commented
Aug 29, 2022
| CONTROL_CHARS = r"[\x00-\x1f\x7f-\xa0]" | ||
| CURATOR_TAG_REGEX = r"(?P<tag>.*)" | ||
| CURATOR_TAG_REGEX = r"(?P<tag>.+)" |
There was a problem hiding this comment.
no empty tags
danieljhegeman
commented
Aug 29, 2022
| @@ -4,23 +4,23 @@ | |||
| ID_REGEX = r"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}" | |||
| DATASET_ID_REGEX = f"(?P<dataset_id>{ID_REGEX})" | |||
| COLLECTION_ID_REGEX = f"(?P<collection_id>{ID_REGEX})" | |||
| CURATOR_TAG_PREFIX_REGEX = r"(?P<tag_prefix>.*)" | |||
danieljhegeman
commented
Aug 29, 2022
| """ | ||
| regex = f"^({DATASET_ID_REGEX}|{CURATOR_TAG_REGEX})$" | ||
| matched = re.match(regex, curator_tag) | ||
| if matched and (tag := matched.groupdict().get("tag")): | ||
| if not re.search(ID_REGEX, tag): |
There was a problem hiding this comment.
removing this because we are only in the business of making sure that we know when to treat an identifier as a uuid, and that is when the identifier takes the precise form of a uuid. Otherwise, if it is not exactly the form of a uuid, we treat it as a curator tag.
There was a problem hiding this comment.
This requirement was explicitly in the ticket when I implemented it. I updated the ticket to reflect the actual requirements.
danieljhegeman
commented
Aug 29, 2022
| @@ -88,7 +88,7 @@ def _test(_tag, _dataset): | |||
| self.assertIsNone(_dataset.curator_tag) | |||
|
|
|||
| dataset = self.generate_dataset(self.session, collection=collection) | |||
| tests = [dataset.id, "prefix" + dataset.id, dataset.id + "suffix", "prefix" + dataset.id + "suffix"] | |||
| tests = [dataset.id, ""] | |||
There was a problem hiding this comment.
(following on from comments above) "prefix" + dataset.id is therefore a valid tag
Bento007
approved these changes
Aug 29, 2022
danieljhegeman
pushed a commit
that referenced
this pull request
Aug 30, 2022
* fix: remove self example from openapi (#3173) * chore: cherry pick changes from #3172 surface dataset batch_condition in curation API (#3177) * feat: surface dataset batch_condition in curation API * resolve merge conflict * fix: document upload links that are suported (#3176) * Document the types of links that are supported * Update documentation * docs: Renaming scExpression to Gene Expression (#3091) * docs: Renaming scExpression to Gene Expression - changing route to gene-expression instead of scExpression * chore(fe): upgrade to nextjs12 blueprint4 node16 (#3119) * chore(fe): upgrade to nextjs12 blueprint4 node16 * upgrade packages to resolve vulns * fix(lint): lint stuff * fix(lint): lint stuff * restore css usage * update Dockerfile * clean up css * fix bp4 css change * update docsite image css * fix: remove access_type from curation API (#3080) * remove access_type form curation API * remove is_allowed check from reshape_for_curation_api * update openapi * Fix the error message when curator is used by not a super curator * fix errors * Move all reshaping collection code into reshape_for_curation_api. * fix: change the datasets response shape (#3178) change name to title in dataset change is_primary_data to list of boolean in dataset * fix(curation api):Remove h5ad suffix requirements (#3151) - remove .h5ad suffix from uploads and curator tags - check for embedded UUID in curator tag * fix: backend/Makefile local db improvements, migration README (#3029) * fix: backend/Makefile local db improvements, migration README - Rename the targets for loading data and loading schema into local test db - Adapt the commands for the above two scripts to actually work (??) * doc updates * additional doc update * style: remove unused regex and consolidate curator tag logic (#3200) Co-authored-by: Trent Smith <1429913+Bento007@users.noreply.github.com> Co-authored-by: Nayib Gloria <55710092+nayib-jose-gloria@users.noreply.github.com> Co-authored-by: ashin-czi <109984998+ashin-czi@users.noreply.github.com> Co-authored-by: Timmy Huang <tihuan@users.noreply.github.com>
danieljhegeman
pushed a commit
that referenced
this pull request
Sep 1, 2022
* fix: remove self example from openapi (#3173) * chore: cherry pick changes from #3172 surface dataset batch_condition in curation API (#3177) * feat: surface dataset batch_condition in curation API * resolve merge conflict * fix: document upload links that are suported (#3176) * Document the types of links that are supported * Update documentation * docs: Renaming scExpression to Gene Expression (#3091) * docs: Renaming scExpression to Gene Expression - changing route to gene-expression instead of scExpression * chore(fe): upgrade to nextjs12 blueprint4 node16 (#3119) * chore(fe): upgrade to nextjs12 blueprint4 node16 * upgrade packages to resolve vulns * fix(lint): lint stuff * fix(lint): lint stuff * restore css usage * update Dockerfile * clean up css * fix bp4 css change * update docsite image css * fix: remove access_type from curation API (#3080) * remove access_type form curation API * remove is_allowed check from reshape_for_curation_api * update openapi * Fix the error message when curator is used by not a super curator * fix errors * Move all reshaping collection code into reshape_for_curation_api. * fix: change the datasets response shape (#3178) change name to title in dataset change is_primary_data to list of boolean in dataset * fix(curation api):Remove h5ad suffix requirements (#3151) - remove .h5ad suffix from uploads and curator tags - check for embedded UUID in curator tag * fix: backend/Makefile local db improvements, migration README (#3029) * fix: backend/Makefile local db improvements, migration README - Rename the targets for loading data and loading schema into local test db - Adapt the commands for the above two scripts to actually work (??) * doc updates * additional doc update * style: remove unused regex and consolidate curator tag logic (#3200) Co-authored-by: Trent Smith <1429913+Bento007@users.noreply.github.com> Co-authored-by: Nayib Gloria <55710092+nayib-jose-gloria@users.noreply.github.com> Co-authored-by: ashin-czi <109984998+ashin-czi@users.noreply.github.com> Co-authored-by: Timmy Huang <tihuan@users.noreply.github.com> Co-authored-by: Trent Smith <1429913+Bento007@users.noreply.github.com> Co-authored-by: Nayib Gloria <55710092+nayib-jose-gloria@users.noreply.github.com> Co-authored-by: ashin-czi <109984998+ashin-czi@users.noreply.github.com> Co-authored-by: Timmy Huang <tihuan@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
.h5adsuffix requirement for identifiers #3102Reviewers
Functional:
@Bento007
Readability:
Changes
QA steps (optional)
Notes for Reviewer