Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Capitalization of collection names should be consistent with TCIA conventions in all places #414

Closed
fedorov opened this issue Oct 17, 2020 · 13 comments
Assignees
Labels
further discussion needed Confirmation of functionality to be confirmed by team. merged:dev Merged into the developement tier production Issue is present in the production tier testing needed in production Can only be tested once in the production tier testing needed Functionality is available in idc-dev for testing
Milestone

Comments

@fedorov
Copy link
Member

fedorov commented Oct 17, 2020

Currently, it is lower-case in the right panel.

image

@fedorov fedorov added the bug Something isn't working label Oct 17, 2020
@fedorov
Copy link
Member Author

fedorov commented Oct 19, 2020

George says it's just formatting, will be fixed to use upper case, consistent with the TCIA conventions.

@ulrikew
Copy link

ulrikew commented Oct 22, 2020

Beware, TCIA uses upper and lower case depending on the collection.
image

@fedorov
Copy link
Member Author

fedorov commented Oct 22, 2020

@ulrikew indeed, so you recommend we use those names? As @bcli4d identified and documented, there are differences in spelling between the various ways those names can be obtained, between TCIA_API_CollectionID, NBIA_CollectionID and TCIA_Webapp_CollectionID.

As a preview, see below, but I can also make a complete list. As you can see, it is not as trivial as using '_' in place of ' '.

Is it a bug or something that need to be addressed on the TCIA/NBIA side to make sure the names are consistent across the various ways those names can be obtained?

image

@fedorov
Copy link
Member Author

fedorov commented Mar 24, 2021

@G-White-ISB @s-paquette can we switch to using tcia_api_collection_id from idc-dev-etl:idc_v2.auxilliary_metadata for the name of the collection displayed in the webapp?

image

As you can see from the summary here: https://docs.google.com/spreadsheets/d/1CZTY8SkPM4mJlSihx-NQ4lZqD5begW67JxpkUjHIyyI/edit?usp=sharing, it's not a simple transformation from idc_webapp_collection_id, and I think it is fair for the user to expect consistency in the names of the collections between IDC and TCIA.

@G-White-ISB G-White-ISB added merged:dev Merged into the developement tier and removed In Progress labels Apr 15, 2021
@G-White-ISB
Copy link

TCIA collection names (https://www.cancerimagingarchive.net/collections/) should now appear consistently in IDC

@fedorov
Copy link
Member Author

fedorov commented Apr 16, 2021

Great, thanks - I see it on dev.

Did you notice that now there are some rendering issues do to lack of wraparound for top-level names?

image

@s-paquette s-paquette added the testing needed Functionality is available in idc-dev for testing label May 12, 2021
@madelyngreyes
Copy link

madelyngreyes commented May 13, 2021

There is another field that has text cut off for count value.

Screen Shot 2021-05-13 at 3 37 26 PM

@ulrikew
Copy link

ulrikew commented May 13, 2021

We have similar issues in NBIA and have brought this up as a topic in this morning's NBIA/TCIA call. Our plan is to work towards a size limit for the collection short names. Which max length would IDC prefer?

@fedorov
Copy link
Member Author

fedorov commented May 13, 2021

I am not sure it is a good idea to change the collection names for the existing collections, so I think we need to have a way to deal with the current lengths. Did you think about enacting the limit going forward, or also for the existing collections, Ulli?

@ulrikew
Copy link

ulrikew commented May 14, 2021

@fedorov I do not yet know the answer to your question. I will put it on the IRCoCo agenda for all to discuss.

@madelyngreyes madelyngreyes added further discussion needed Confirmation of functionality to be confirmed by team. and removed testing needed Functionality is available in idc-dev for testing labels Jun 7, 2021
@s-paquette s-paquette removed bug Something isn't working merged:dev Merged into the developement tier labels Aug 16, 2021
@G-White-ISB
Copy link

Old issue but worth checking again? I think we are now consistent

@G-White-ISB G-White-ISB added the testing needed in production Can only be tested once in the production tier label Jan 21, 2022
@s-paquette s-paquette added merged:dev Merged into the developement tier testing needed Functionality is available in idc-dev for testing and removed testing needed in production Can only be tested once in the production tier labels Feb 7, 2022
@s-paquette s-paquette added this to the Release 26 milestone Feb 7, 2022
@s-paquette s-paquette added the testing needed in production Can only be tested once in the production tier label Feb 7, 2022
@s-paquette
Copy link
Member

@fedorov @ulrikew Do we now meet the requirements of this ticket?

@ulrikew
Copy link

ulrikew commented Feb 15, 2022

Yes, looks good. Only (very minor) difference I found was the last letter in the MIDRC collection names. Don't fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
further discussion needed Confirmation of functionality to be confirmed by team. merged:dev Merged into the developement tier production Issue is present in the production tier testing needed in production Can only be tested once in the production tier testing needed Functionality is available in idc-dev for testing
Projects
None yet
Development

No branches or pull requests

5 participants