Added basic API for Column-level attributes (issue #821) #893

jondoesntgit · 2021-03-19T22:05:51Z

The column-level attributes are only accessible once the table and the columns are instantiated. There's currently no support for adding attributes on creation (e.g., something like pt.Float32Col(dflt=0.0, shape=(3,2), attrs=dict(foo='bar', description='some string'))

avalentino

In general the PR looks good but there are few things to address.
Also there are two important things missing:

unit tests
documentation update

tables/attributeset.py

tables/table.py

tables/attributeset.py

mennthor · 2021-12-15T07:21:11Z

Hi :)
What is missing / needs to be done to make this merged?

I'm creating IsDescription classes by wrapper functions to set column lengths for 1 Bit serialised bool arrays, so they take less space with np.packbits and tables.StrCol(int(np.ceil(NBITS / 8.)).
It would be great to store the number of Bits directly in the method where I'm creating the table IsDescription subclass, because I rely on that information to be present in the table attrs to properly truncate the array when deserialising after reading the table back in.

avalentino

The implementation looks good to me.
Please add

at least basic unit tests
basic documentation
a changelog entry

tables/table.py

avalentino · 2021-12-15T07:42:46Z

Sorry @jondoesntgit for the long delay to provide a feedback.
The PR seems to be almost ready for merge.

One call to H5*ARRAYmake() used completely different indentation style.

hdf5 doesn't like that: h5py/h5py#1948 HDFGroup/hdf5@16349c5 > The 'not suitable for filters' message does seem to have been added > in 1.12.1 From the code, it looks like it will refuse to apply any > filters to vlen strings. This patch is based on the assumption that the hdf5 code is *correct*, and indeed fletcher32 shouldn't be used in that vlarrays. But if hdf5 is wrong, then the fix should be their side. It might make sense to apply this to get the tests passing again, even if ultimately hdf5 is adjusted too. Fixes PyTables#845.

Add python3.10 classifier

This enables to attach attributes during the assembly of a IsDescription class. Example: ``` class TableLayout(tables.IsDescription): freq = tables.Float32Col( attrs={"val": 13.3, "unit": "Hz", "description": "Ref. freq"}) ```

…l_attrs This registers the Col._v_col_attrs in the ColumnAttributeSet and stores them as table attributes. This way column attributes set in the IsDescription assembly are propagated to the Column attributes.

Otherwise the attributes are only build when the column is used directly for attribute assignment. This then does not store the attributes inserted in the IsDescription class.

jondoesntgit · 2021-12-17T01:59:49Z

No worries on the delay. I'd like to give some time this December to this, but there's a lot of other things competing for my time (new family member, wrapping up PhD, etc...)

…

On Tue, Dec 14, 2021 at 11:42 PM Antonio Valentino ***@***.***> wrote: Sorry @jondoesntgit <https://github.com/jondoesntgit> for the long delay to provide a feedback. The PS seems to be almost ready for merge. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#893 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABZCG2BNCV3FQI3ZXXFML53URBBIFANCNFSM4ZPSVPSQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

Zybulon · 2022-12-17T09:03:31Z

Hi,
This feature is a great idea, I am very interesting in it to store units and description (like you did I guess).
Is there still a lot a work to do ? The requested changes still need to be done ?

mennthor · 2022-12-18T08:37:36Z

@Zybulon I adressed the original leftover tasks quite a while ago, but I'm not sure where this currently stands

avalentino · 2022-12-18T11:36:18Z

Could you please push new changes in this PR?
I hope that @jondoesntgit doesn't mind it.

Zybulon · 2023-01-03T10:56:34Z

I am not very familiar with Github so I might misunderstand some things. @jondoesntgit does not seems very active, so I guess we can't push code in this PR, am I wrong ? Maybe @mennthor can create a new PR to push its modifications ?
Edit : sorry I confused your names

jondoesntgit · 2023-01-03T15:07:02Z

Hello, Sorry for the late reply. Things got a little crazy over the holidays. I think when I left off, I just needed to write some unit tests and documentation. I’d love if someone took over this PR.

…

Sent from my iPhone

On Jan 3, 2023, at 05:56, Zybulon ***@***.***> wrote: I am not very familiar with Github so I might misunderstand some things. @avalentino does not seems very active, so I guess we can't push code in this PR, am I wrong ? Maybe @mennthor can create a new PR to push its modifications ? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

Zybulon · 2023-01-08T12:50:53Z

@jondoesntgit I think you can accept menthor's commit in your fork, can you do it ? It should be immediate since there is not conflict.

Requested updates to merge PyTables PR PyTables#893

jondoesntgit · 2023-01-10T03:51:28Z

Is git rebase the right move at this point?

avalentino · 2023-01-10T06:14:22Z

Is git rebase the right move at this point?

you can re-base or merge master into your branch

jondoesntgit · 2023-05-31T14:57:23Z

Back in January, I did a rebase, and then I just did a merge a few minutes ago. I'm sorry that this has taken so long.

Let me know if there's any issues with merging this into master. I'm needing to factory-reset my laptop, and this is the last thing I'm waiting for before I factory reset.

avalentino · 2023-05-31T15:59:20Z

Dear @jondoesntgit, thanks a lot for you effort.
Could you please just remove the appveyor.yml file?
I think that ir was removed in master after your initial PR.

jondoesntgit · 2023-05-31T17:00:00Z

@avalentino Done

avalentino · 2023-05-31T19:28:35Z

Thanks again @jondoesntgit

Added basic API for Column-level attributes (issue PyTables#821)

707f13d

avalentino requested changes Mar 20, 2021

View reviewed changes

jondoesntgit added 2 commits March 25, 2021 16:59

Addressed most of the comments

afe1951

Move code to table.py to avoid circular dependency

95b1af8

avalentino requested changes Dec 15, 2021

View reviewed changes

avalentino reviewed Dec 15, 2021

View reviewed changes

tables/table.py Outdated Show resolved Hide resolved

avalentino reviewed Dec 15, 2021

View reviewed changes

tables/table.py Outdated Show resolved Hide resolved

zequihg50 and others added 21 commits December 16, 2021 23:15

Enhancement for Index

fa91c95

Add linux aarch64 wheel support

a4e2585

convert np.typeDict to np.sctypeDict

b00e361

Do not use PyArray_DATA in nogitl sections

cace9e8

Add 'import ctypes.util'

5bde35f

pyx: adjust indentation to match surrounding calls

0082489

One call to H5*ARRAYmake() used completely different indentation style.

Fix two typos

19b87e9

Remove duplicate test

ee050d1

Do not flush when read only

101df8c

Do not build wheels for musllinux

b9d21a2

Update links to CI in the project pointers page

b68f92d

Fix broken links

1c7adb2

Fix compatibility with numpy 1.20

3aaad24

Run CI against python 3.10

f0e6af5

Add python3.10 classifier

Build linux/MacOS wheels for python 3.10

2d97ca6

Don't build wheels for PR branch

b43e787

Remove configuration file for Travis-CI

6845283

Switch to conda-incubator/setup-miniconda

e4b0977

Do not install doc dependencies

0227676

Add conda-forge channel

d9b2304

xmatthias and others added 6 commits December 16, 2021 23:15

Update build matrix

028dde4

build macos ARM wheels

e41228b

Added support for attributes in the Col class.

344c7ef

This enables to attach attributes during the assembly of a IsDescription class. Example: ``` class TableLayout(tables.IsDescription): freq = tables.Float32Col( attrs={"val": 13.3, "unit": "Hz", "description": "Ref. freq"}) ```

Extends the Column and ColumnAttributeSet to be agnostic of Col._v_co…

f68d89f

…l_attrs This registers the Col._v_col_attrs in the ColumnAttributeSet and stores them as table attributes. This way column attributes set in the IsDescription assembly are propagated to the Column attributes.

Place ColumnAttributeSet instantiation in the Column __init__

ba92d9e

Otherwise the attributes are only build when the column is used directly for attribute assignment. This then does not store the attributes inserted in the IsDescription class.

Added unit tests for

1abf377

Added basic documentation for the Column attributes

71e7d00

mennthor mentioned this pull request Dec 17, 2021

Requested updates to merge PyTables PR #893 jondoesntgit/PyTables#1

Merged

Suggested change: Exception to TypeError

8804d24

avalentino added this to the Next Tasks milestone Dec 28, 2021

Merge pull request #1 from mennthor/master

d07eef2

Requested updates to merge PyTables PR PyTables#893

jondoesntgit added 3 commits January 12, 2023 07:57

Added a blurb in docs

af152a5

Changed version in documentation

53688b3

Merge remote-tracking branch 'upstream/master'

acc27ca

Removed appveyor.yml per @avalentino's request

0e30264

avalentino approved these changes May 31, 2023

View reviewed changes

avalentino merged commit f8ad2f1 into PyTables:master May 31, 2023
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added basic API for Column-level attributes (issue #821) #893

Added basic API for Column-level attributes (issue #821) #893

jondoesntgit commented Mar 19, 2021

avalentino left a comment

mennthor commented Dec 15, 2021

avalentino left a comment

avalentino commented Dec 15, 2021 •

edited

jondoesntgit commented Dec 17, 2021 via email

Zybulon commented Dec 17, 2022

mennthor commented Dec 18, 2022

avalentino commented Dec 18, 2022

Zybulon commented Jan 3, 2023 •

edited

jondoesntgit commented Jan 3, 2023 via email

Zybulon commented Jan 8, 2023

jondoesntgit commented Jan 10, 2023

avalentino commented Jan 10, 2023

jondoesntgit commented May 31, 2023

avalentino commented May 31, 2023

jondoesntgit commented May 31, 2023

avalentino commented May 31, 2023

Added basic API for Column-level attributes (issue #821) #893

Added basic API for Column-level attributes (issue #821) #893

Conversation

jondoesntgit commented Mar 19, 2021

avalentino left a comment

Choose a reason for hiding this comment

mennthor commented Dec 15, 2021

avalentino left a comment

Choose a reason for hiding this comment

avalentino commented Dec 15, 2021 • edited

jondoesntgit commented Dec 17, 2021 via email

Zybulon commented Dec 17, 2022

mennthor commented Dec 18, 2022

avalentino commented Dec 18, 2022

Zybulon commented Jan 3, 2023 • edited

jondoesntgit commented Jan 3, 2023 via email

Zybulon commented Jan 8, 2023

jondoesntgit commented Jan 10, 2023

avalentino commented Jan 10, 2023

jondoesntgit commented May 31, 2023

avalentino commented May 31, 2023

jondoesntgit commented May 31, 2023

avalentino commented May 31, 2023

avalentino commented Dec 15, 2021 •

edited

Zybulon commented Jan 3, 2023 •

edited