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

Merge data-file-summarizer into overviewpy #17

Open
wants to merge 12 commits into
base: feature-summarizer
Choose a base branch
from

Conversation

ba66e77
Copy link

@ba66e77 ba66e77 commented Jul 28, 2023

As discussed in Issue #14, moving the functionality of data-file-summarizer into overviewpy.

@ba66e77
Copy link
Author

ba66e77 commented Jul 28, 2023

This PR is a work in progress. Additional work is needed to:

  • Ensure the dependencies of data-file-summarizer are installed when overviewpy is installed through pip.
  • Rearrange code for consistent organization of the project files.
  • Provide a way to access the summarizer feature from inside Python (i.e., don't make people drop back to a command line to generate the summary).
  • Standardize naming conventions (e.g., pd vs pandas aliasing)

@ba66e77 ba66e77 marked this pull request as ready for review July 28, 2023 22:35
@ba66e77
Copy link
Author

ba66e77 commented Jul 28, 2023

Ok, I've realized there already is a code method to access the summarizer without dropping back to the command line. So I'm caling that one done.

For the last one, I kind of mislove the use of pd as the alias for pandas.

I propose merging the PR as it is and we can adjust code style and stuff after the fact.

@ba66e77
Copy link
Author

ba66e77 commented Jul 29, 2023

Asking around a bit on masstodon, seems like most of the community sees no problem with aliasing pandas as pd. So, since the community is down with it and the rest of the package already uses that nomenclature, I bit the bullet and updated the summarizer to use that convention also.

@cosimameyer cosimameyer changed the base branch from main to feature-summarizer July 31, 2023 13:44
@cosimameyer
Copy link
Owner

Thanks again for the PR!

I changed the target branch - this way we can merge all upcoming PRs to an up-to-date base feature branch that mirrors the main branch, and make sure all changes are done as expected before moving them back to main and releasing them :)

Just wondering because there seem to be issues with poetry. What OS are you developing on? I use a docker image for the project (sorry for not mentioning it earlier) - if you feel comfortable with that, it might be good to have that as a common base.

@ba66e77
Copy link
Author

ba66e77 commented Jul 31, 2023

I'm developing on OSX in a conda environment with python 3.10.

What kind of problems are you seeing with poetry?

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

Successfully merging this pull request may close these issues.

2 participants