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

Small tweaks to catalog #1100

Closed
alasarr opened this issue Oct 13, 2019 · 7 comments · Fixed by #1105
Closed

Small tweaks to catalog #1100

alasarr opened this issue Oct 13, 2019 · 7 comments · Fixed by #1105
Assignees

Comments

@alasarr
Copy link
Contributor

alasarr commented Oct 13, 2019

[x] 1. Return slug at every object with slug. For example, Dataset.slug doesn't work but it appears at dataset.to_dict. We talked about removing slug for the model but I'm regretting it after I've played a little bit with the library.

[x] 2. Dataset.to_dict(): remove summary_jsonb.

[x] 3. Rename Catalog to CatalogDataset

[ ] 4. Custom __repr__ for Variable objet.

image

That would be great to include only for Variable object the description fields at the repr.

For longer description (bigger than 20 characters let's add an ellipsis.

<Variable('CRMCYPERC_1a97ab94', 'My cool short description')>
<Variable('CRMCYPERC_1a97ab94', 'My cool long description bla bla bla ...' )>

@esloho
Copy link
Contributor

esloho commented Oct 14, 2019

  1. Return slug at every object with slug. For example, Dataset.slug doesn't work but it appears at dataset.to_dict. We talked about removing slug for the model but I'm regretting it after I've played a little bit with the library.

I finally added the slug property at #1093 since I needed it for the CatalogList.get(slug) to work :)

@esloho
Copy link
Contributor

esloho commented Oct 14, 2019

2. Dataset.to_dict(): remove summary_jsonb.

I assume we want that for all entities with a summary_jsonb field (like Variable or Geography), don't we @alasarr ?

@alasarr
Copy link
Contributor Author

alasarr commented Oct 14, 2019

I assume we want that for all entities with a summary_jsonb field (like Variable or Geography), don't we @alasarr ?

Yes, but only for to_dict the property should be available for describe

@esloho
Copy link
Contributor

esloho commented Oct 14, 2019

3. Rename Catalog to CatalogDataset

I think this will be included as part of #1018 . Should we handle it now or wait for that issue? @alrocar @oleurud

@alrocar
Copy link
Contributor

alrocar commented Oct 14, 2019

As for know I'd do what @alasarr requests, since these changes may affect directly the demo notebook and he's the owner of it.

@esloho
Copy link
Contributor

esloho commented Oct 14, 2019

Perfect, I'll change it here then 👍

@esloho
Copy link
Contributor

esloho commented Oct 14, 2019

For longer description (bigger than 20 characters let's add an ellipsis.

<Variable('CRMCYPERC_1a97ab94', 'My cool short description')>
<Variable('CRMCYPERC_1a97ab94', 'My cool long description bla bla bla ...' )>

Maybe we should make it for longer than 30 characters or so (even the string 'My cool short description" exceeds the 20 characters limit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants