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

[CT-1350] Clean up string formatting #6068

Closed
emmyoop opened this issue Oct 14, 2022 · 10 comments
Closed

[CT-1350] Clean up string formatting #6068

emmyoop opened this issue Oct 14, 2022 · 10 comments
Assignees
Labels
good_first_issue Straightforward + self-contained changes, good for new contributors! Hackathon Coalesce Hackathon Submissions stale Issues that have gone stale tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality

Comments

@emmyoop
Copy link
Member

emmyoop commented Oct 14, 2022

Describe the feature

Clean up the strings in dbt-core.

When we originally mass formatted our code with black, it caused some weird formatting that does not matter to Python but is not how it should be.

Examples:

raise NotImplementedException(
"`list_relations_without_caching` is not implemented for this " "adapter!"
)

raise RuntimeException(
f"Could not find selector named {name}, expected one of " f"{list(self.selectors)}"
)

Searching the code for " f" finds many. The first example is harder to find as there are legitimate parts of the code with empty strings. It is still doable however.

Describe alternatives you've considered

Doing it as we touch these areas of code.

Who will this benefit?

Anyone looking at the code who is irked by this formatting.

Are you interested in contributing this feature?

No response

Anything else?

No response

@emmyoop emmyoop added the tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality label Oct 14, 2022
@github-actions github-actions bot changed the title Clean up string formatting [CT-1350] Clean up string formatting Oct 14, 2022
@emmyoop emmyoop added the good_first_issue Straightforward + self-contained changes, good for new contributors! label Oct 14, 2022
@emmyoop emmyoop assigned emmyoop and unassigned emmyoop Oct 17, 2022
@emmyoop emmyoop added the Hackathon Coalesce Hackathon Submissions label Oct 17, 2022
@mhoss2008
Copy link

Hi

@emmyoop emmyoop assigned mhoss2008 and unassigned emmyoop Oct 17, 2022
@luke-bassett
Copy link
Contributor

Can I help with this one?

@luke-bassett
Copy link
Contributor

I think this command finds all the files that contain cases that match the examples in the first comment on this issue (with a few false positives):

fd '.py$' | xargs grep -Ei '(.*"\s+".*)|(.*"\s+f".*)' | awk '{print $1}' | sort | uniq | sed -E 's/(.*):/\1/'
./core/dbt/adapters/base/impl.py
./core/dbt/clients/_jinja_blocks.py
./core/dbt/config/project.py
./core/dbt/context/base.py
./core/dbt/context/providers.py
./core/dbt/contracts/connection.py
./core/dbt/contracts/graph/manifest.py
./core/dbt/contracts/graph/unparsed.py
./core/dbt/contracts/results.py
./core/dbt/events/functions.py
./core/dbt/events/types.py
./core/dbt/exceptions.py
./core/dbt/graph/cli.py
./core/dbt/graph/graph.py
./core/dbt/graph/selector.py
./core/dbt/logger.py
./core/dbt/parser/base.py
./core/dbt/parser/generic_test_builders.py
./core/dbt/parser/schemas.py
./core/dbt/parser/sources.py
./core/dbt/task/base.py
./core/dbt/task/compile.py
./core/dbt/task/freshness.py
./core/dbt/task/init.py
./core/dbt/task/runnable.py
./core/dbt/tests/util.py
./core/dbt/utils.py
./plugins/postgres/dbt/adapters/postgres/connections.py
./tests/functional/artifacts/expected_manifest.py
./tests/functional/configs/test_disabled_model.py
./tests/functional/deprecations/test_deprecations.py
./tests/functional/duplicates/test_duplicate_analysis.py
./tests/functional/duplicates/test_duplicate_macro.py
./tests/functional/duplicates/test_duplicate_model.py

@mhoss2008 mhoss2008 removed their assignment Oct 17, 2022
@luke-bassett
Copy link
Contributor

Working on this one for the hackathon

@eve-johns
Copy link
Contributor

Working on this one for the hackathon

+1

@emmyoop
Copy link
Member Author

emmyoop commented Oct 17, 2022

@luke-bassett script is a great way to go! The " " case has a lot more false positives. Feel free to do this in two iterations - once for f-strings and once for plain string.

@luke-bassett
Copy link
Contributor

Looks like @eve-johns has covered f-strings. This command still has a couple false positives, but fewer.

fd '.py$' | xargs grep -Ei '.*".*"\s+".*".*' | awk '{print $1}' | sort | uniq | sed -E 's/(.*):/\1/' 

I'm making the changes now.

@eve-johns
Copy link
Contributor

Ah I didn't noticed there are also plain strings problem. Trying regx

leahwicz added a commit that referenced this issue Oct 17, 2022
* fix f string issue

* removed one space

* Add changelog

* fixed return format

Co-authored-by: Leah Antkiewicz <leah.antkiewicz@fishtownanalytics.com>
@jtcohen6 jtcohen6 modified the milestone: v1.4 Dec 6, 2022
Copy link
Contributor

github-actions bot commented Dec 2, 2023

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please comment on the issue or else it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label Dec 2, 2023
Copy link
Contributor

github-actions bot commented Dec 9, 2023

Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest. Just add a comment to notify the maintainers.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good_first_issue Straightforward + self-contained changes, good for new contributors! Hackathon Coalesce Hackathon Submissions stale Issues that have gone stale tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality
Projects
None yet
Development

No branches or pull requests

5 participants