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

feat: Rework import generation for stubs. #50

Merged
merged 58 commits into from
Feb 25, 2024

Conversation

Masara
Copy link
Contributor

@Masara Masara commented Dec 10, 2023

Closes #38
Closes #24

Summary of Changes

Other changes

  • Added a "// Todo" for the generated Stubs, if an internal class is used as a type.
  • Added "// Todo Safe-DS does not support set types" if a set object is used.
  • If booleans where used in Literal Types the first letter was capitalized in stubs, which it should not be.
  • Removed the "where" part and lower limits of the Stubs for constrains

@Masara Masara self-assigned this Dec 10, 2023
@Masara Masara requested a review from a team as a code owner December 10, 2023 14:51
Copy link

codecov bot commented Dec 10, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.76%. Comparing base (ced63f3) to head (47077b6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
+ Coverage   98.26%   98.76%   +0.49%     
==========================================
  Files          25       25              
  Lines        1905     2017     +112     
==========================================
+ Hits         1872     1992     +120     
+ Misses         33       25       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Dec 10, 2023

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 16 0 0 1.38s
✅ PYTHON mypy 16 0 5.96s
✅ PYTHON ruff 16 0 0 0.04s
✅ REPOSITORY git_diff yes no 0.02s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@Masara
Copy link
Contributor Author

Masara commented Dec 10, 2023

I realized that changes in this PR clash with what we need from Issue #38. Therefore I will revert this PR to a draft and use it as a PR for issue #38.

@Masara Masara marked this pull request as draft December 10, 2023 19:55
@Masara Masara changed the title fix: Ignore parameter, result and attribute types from outside packages feat: Rework import generation for stubs. Dec 10, 2023
@Masara Masara marked this pull request as ready for review February 21, 2024 10:29
@Masara
Copy link
Contributor Author

Masara commented Feb 21, 2024

I'm done with this PR now, too @lars-reimann. Could you check this?

@lars-reimann
Copy link
Member

Yes, probably tomorrow. Could you add a summary of changes?

@lars-reimann
Copy link
Member

lars-reimann commented Feb 21, 2024

We also need a syrupy extension and a pytest fixture for stubs (see this example). This way all created stubs get their own file with the correct extension, instead of mixing them all together in one ambr file.

Masara and others added 3 commits February 22, 2024 14:08
# Conflicts:
#	tests/conftest.py
#	tests/safeds_stubgen/stubs_generator/__snapshots__/test_generate_stubs.ambr
#	tests/safeds_stubgen/stubs_generator/test_generate_stubs.py
@Masara Masara marked this pull request as draft February 23, 2024 13:16
@Masara Masara marked this pull request as ready for review February 23, 2024 19:17
@Masara
Copy link
Contributor Author

Masara commented Feb 23, 2024

@lars-reimann I fixed how the import paths are generated for stubs, now it should be correct. There was just one case where I wasn't sure how to handle it: If we have an import from another package, how should it look like?
This

@PythonModule("various_modules_package.function_module")
package variousModulesPackage.functionModule

from tests.data.main_package.another_path.another_module import AnotherClass

or this

@PythonModule("various_modules_package.function_module")
package variousModulesPackage.functionModule

from mainPackage.AnotherPath.AnotherModule import AnotherClass

?

Currently the first case is implemented. If it's correct like this, you can review and hopefully close this PR.

@lars-reimann
Copy link
Member

lars-reimann commented Feb 23, 2024

The second option is correct, assuming the file containing AnotherClass looks something like this:

@PythonModule("does not matter for imports in Safe-DS")
package mainPackage.AnotherPath.AnotherModule

class AnotherClass

The part between from and import must always be equal to the name of the Safe-DS package you want to import from.

@Masara
Copy link
Contributor Author

Masara commented Feb 24, 2024

@lars-reimann But we are assuming that we did not create the "mainPackage" Stubs yet and don't know where the mainPackage files are. What I concretely mean is, how do imports look like, if we import classes from other packages? Packages we perhaps did not create stubs for? The problem here is that we don't know how the package is named, like in the tests.data.main_package.another_path.another_module.AnotherClass example. Mypy gives us this path to the class, but we actually don't now that "main_package" is the package. Or should I just assume that because our current package, which we are creating stubs for, has the path tests.data.out_current_package.... that we can slice of the tests.data part for tests.data.main_package...., too?

@lars-reimann
Copy link
Member

lars-reimann commented Feb 24, 2024

Say we are creating stubs for the package a and are currently working on a Python module with the following content:

from b import SomeClass

def f(p: SomeClass)

Then we also need to create some stub for the class SomeClass, or we can't use it. So limited stubs must be created for the declarations of other packages that package a depends on.

For this PR it's sufficient to completely remove such imports, but please create an issue that stubs must be created for referenced declarations in other packages.

@Masara
Copy link
Contributor Author

Masara commented Feb 24, 2024

@lars-reimann If I remove those imports for this PR, what about the types that use the imported classes? Should I removed them, too, for the time being?

@lars-reimann
Copy link
Member

No, keep the types.

@Masara
Copy link
Contributor Author

Masara commented Feb 24, 2024

@lars-reimann I removed the imports and created issue #66. You can proceed with the PR review.

@lars-reimann lars-reimann merged commit 216e179 into main Feb 25, 2024
8 checks passed
@lars-reimann lars-reimann deleted the ignore_types_from_outside_packages branch February 25, 2024 09:01
lars-reimann pushed a commit that referenced this pull request Mar 29, 2024
## [0.2.0](v0.1.0...v0.2.0) (2024-03-29)

### Features

* Added generation for Safe-DS stubs files ([#33](#33)) ([ab45b45](ab45b45))
* Correct stubs for TypeVars ([#67](#67)) ([df8c5c9](df8c5c9)), closes [#63](#63)
* Create stubs for public methods of inherited internal classes ([#69](#69)) ([71b38d7](71b38d7)), closes [#64](#64)
* Rework import generation for stubs. ([#50](#50)) ([216e179](216e179)), closes [#38](#38) [#24](#24) [#38](#38) [#24](#24)
* Safe-DS stubs also contain docstring information. ([#78](#78)) ([bdb43bd](bdb43bd))
* Stubs are created for referenced declarations in other packages ([#70](#70)) ([522f38d](522f38d)), closes [#66](#66)

### Bug Fixes

* Some packages couldn't be analyzed ([#51](#51)) ([fa3d020](fa3d020)), closes [#48](#48)
* Stub generation testing and fixing of miscellaneous bugs ([#76](#76)) ([97b0ab3](97b0ab3))
@lars-reimann
Copy link
Member

🎉 This PR is included in version 0.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@lars-reimann lars-reimann added the released Included in a release label Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Included in a release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Rework import generation for stubs. feat: Resolve aliasing for imports
3 participants