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

CollectionRowBlock property name inconsistency #4

Closed
ivanistheone opened this issue Dec 19, 2018 · 5 comments
Closed

CollectionRowBlock property name inconsistency #4

ivanistheone opened this issue Dec 19, 2018 · 5 comments
Labels
enhancement New feature or request help wanted Contributions and suggestions welcome

Comments

@ivanistheone
Copy link
Contributor

I'm working with a collection, and I'd like to know the key names (properties) for the rows in that collection. Consider the following helper methods:

def _print_keys(coll):
    """
    One way to get properties keys for a collection.
    """
    first_row = coll.get_rows()[0]
    return list(first_row.get_all_properties().keys())

def _print_keys_alt(coll):
    """
    Another way to get properties keys for a collection.
    """
    props = pipeline_coll.get_schema_properties()
    return [prop['slug'] for prop in props]

When running on the Chef Pipeline board collection, I get the following outputs:

>>> pipeline_coll = client.get_collection('c70d925f-fd8f-4134-a26b-0b71a14bd6ff')

>>> _print_keys(pipeline_coll)
['license(s)',
 'source_url',
 ...

>>> _print_keys_alt(pipeline_coll)
['license_s',
 'source_url',
  ...

Expected

first_row.get_all_properties().keys() to be the same as slug or name of the property.

Observed

The key license(s) seems like lowercase version of the property name

Context

I was trying to find the property "accessor" method for getting properties and tried all of these options:

    row.get('license(s)'),  # None
    row.get('License(s)'),  # None
    row.get('license_s'),  # None
    row.get(['license(s)']),  # None
    row.get(['License(s)']), # None
    row.get(['license_s']),  # None
    row['license(s)'],  # raises since no __getitem__
    row['License(s)'],  # raises since no __getitem__
    row['license_s'],  # raises since no __getitem__

before learning about the row.get_property method which works with all three identifiers:

    row.get_property('license(s)'),  # GOOD
    row.get_property('License(s)'),  # GOOD
    row.get_property('license_s'),   # GOOD

@jamalex Would it make sense to define a get and/or __getitem__ as alias for get_property for CollectionRowBlock objects?

@jamalex jamalex added the enhancement New feature or request label May 8, 2019
@jamalex
Copy link
Owner

jamalex commented May 8, 2019

Note that the slugified name can already be accessed as a property: row.license_s. But that's not amenable to passing in the non-slugified name. But adding __getitem__ could be good too, if you would find that idiom useful! I don't see any issues with it, and it would allow use of versions of the name that can't be used as property names directly.

PR's welcome for this! (see below)

@jamalex jamalex added the help wanted Contributions and suggestions welcome label May 8, 2019
@jamalex
Copy link
Owner

jamalex commented May 8, 2019

Another note: row.get("...") is very different from row.get_property. The data structure for a page (in this case, a collection row) is something like:

In [22]: row.get()                                                                                                                                                                                                                             
Out[22]: 
{'alive': True,
 'created_by': 'f86b4884-75fd-41ba-9161-2fad359c775e',
 'created_time': 1545009470407,
 'id': 'd7f9a19a-b595-49c0-a2ae-58e7d5248a67',
 'last_edited_by': 'f86b4884-75fd-41ba-9161-2fad359c775e',
 'last_edited_time': 1557276849563,
 'parent_id': '02a32a6b-aee3-4868-b2f9-01da7ec88686',
 'parent_table': 'collection',
 'properties': {'f=DE': [['QQQ']], 'title': [['My title']], 'vFY4': [['No']]},
 'type': 'page',
 'version': 22}

The get method reads data from this top-level dict. So you can do row.get("version") to get 22. The get_property method, on the other hand, pokes inside the properties subkey, and provide a bunch of assistance with making it less painful (converting human-readable property names into the underlying 4-character "property ID", converting internal value data formats into Pythonic data, etc). So I think it would actually be best to avoid adding additional interfaces onto these data structures, since it might just compound the complexity.

@jamalex
Copy link
Owner

jamalex commented May 8, 2019

Instead, I'll go back and fix the original issue being reported here. :)

It looks as if get_all_properties didn't do full slugification, which is the source of the inconsistency. Fixing that shortly!

@jamalex
Copy link
Owner

jamalex commented May 8, 2019

Fixed in #23, which will shortly be released as 0.0.21!

@jamalex
Copy link
Owner

jamalex commented May 8, 2019

Version 0.0.21 has now been released to PyPI! Please give it a try and see how it works for you. Thanks for reporting issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Contributions and suggestions welcome
Projects
None yet
Development

No branches or pull requests

2 participants