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

Hide all type-hint-only imports behind if TYPE_CHECKING #2992

Merged
merged 6 commits into from May 16, 2023
Merged

Conversation

janosh
Copy link
Member

@janosh janosh commented May 16, 2023

4 main motivations for this change:

  1. reduces chance of circular imports
  2. can help contributors run tests without installing extra deps like pymatgen.analysis.defects if they were previously in a test's import stack but only used for type hints
  3. helps with readability by clearly separating type-only imports
  4. can decrease startup time since TYPE_CHECKING=False at run time means some modules won't have to be imported. somewhat negated by the annoyingly long import time of the std lib typing module itself but that has been steadily coming down with newer Python versions.

@janosh janosh added linting Linting and quality assurance types Type all the things imports Import changes and formatting labels May 16, 2023
@janosh janosh enabled auto-merge (squash) May 16, 2023 15:10
@janosh
Copy link
Member Author

janosh commented May 16, 2023

Ci failure is due to flaky test:

    @unittest.skipIf(
        not SETTINGS.get("PMG_MAPI_KEY") or website_down,
        "PMG_MAPI_KEY environment variable not set or MP is down.",
    )
    def test_get_snls_mp(self):
        with OptimadeRester("mp") as optimade:
            structs = optimade.get_snls(elements=["Ga", "N"], nelements=2)
    
        with OptimadeRester("mp") as optimade:
            response_field_structs_single = optimade.get_snls(
                elements=["Ga", "N"], nelements=2, additional_response_fields="nsites"
            )
            response_field_structs_set = optimade.get_snls(
                elements=["Ga", "N"], nelements=2, additional_response_fields={"nsites", "nelements"}
            )
            if ("mp" in response_field_structs_single) and ("mp" in response_field_structs_set):
>               assert len(structs["mp"]) == len(response_field_structs_single["mp"])
E               KeyError: 'mp'

pymatgen/ext/tests/test_optimade.py:55: KeyError

@janosh janosh disabled auto-merge May 16, 2023 15:40
@janosh janosh merged commit 6c9ddef into master May 16, 2023
39 of 40 checks passed
@janosh janosh deleted the ruff-tch branch May 16, 2023 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imports Import changes and formatting linting Linting and quality assurance types Type all the things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant