Skip to content
This repository was archived by the owner on Jul 31, 2023. It is now read-only.

Conversation

@mbernico
Copy link
Contributor

This should close PR #15 Please take a look @cfezequiel @jmarrietar

@google-cla
Copy link

google-cla bot commented Aug 14, 2020

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@mbernico mbernico requested a review from cfezequiel August 14, 2020 14:26
@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@jmarrietar
Copy link
Contributor

jmarrietar commented Aug 15, 2020

Hi @mbernico @cfezequiel ,

I want to take this PR to maybe raise a related issue.

What worked for me to create the TFRecords was creating them with the pandas DataFrame accessor df.tensorflow.to_tfr(output_dir=OUTPUT_PATH), because I think it uses from tfrecorder import client inside accessor.py.

Using the example in the README file raised me an error of AttributeError: module 'tfrecorder' has no attribute 'create_tfrecords' that might be related to the __init__.py file.

If I add the following to the __init__.py file it recognized the methods as shown in the README.md

from .client import create_tfrecords
from .check import check_tfrecords

I took as reference this article https://towardsdatascience.com/whats-init-for-me-d70a312da583 but not sure if maybe a problem from my side this behavior. I tried to reproduce the error installing from the code in a new virtualenv and the same behavior exists.

Thanks!,

@cfezequiel
Copy link
Contributor

Hi @jmarrietar , thanks for the feedback! We normally try to avoid . imports and importing classes/functions directly as part of our coding style, but I did notice the AttributeError you mentioned with the pip-installed tfrecorder in its current state, so that's an issue.
I think your suggestion for modifying __init__.py makes sense, especially importing just the desired functions. I would however change the . import to the following for consistency with other imports:

from tfrecorder.client import create_tfrecords
from tfrecorder.check import check_tfrecords

I've tested the above with pip-installed tfrecorder but let me know if you encounter issues with it.

@jmarrietar
Copy link
Contributor

Hi @cfezequiel , yes with that change there is no AttributeError. I closed #15 Cheers,

@mbernico mbernico merged commit f989f23 into master Aug 21, 2020
@mbernico mbernico deleted the pr/15 branch August 21, 2020 21:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants