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

fix: The file structure of stubs resembles the "package" path. #106

Merged
merged 15 commits into from
May 2, 2024

Conversation

Masara
Copy link
Contributor

@Masara Masara commented Apr 7, 2024

Closes #81

Summary of Changes

Changed how the file structure for stubs is build. Now we follow the "package" path for the structure.

@Masara Masara requested a review from a team as a code owner April 7, 2024 12:59
@Masara Masara linked an issue Apr 7, 2024 that may be closed by this pull request
Copy link

github-actions bot commented Apr 7, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 7 0 0 1.26s
✅ PYTHON mypy 7 0 5.6s
✅ PYTHON ruff 7 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

Copy link

codecov bot commented Apr 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.53%. Comparing base (660ac0d) to head (5543683).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #106   +/-   ##
=======================================
  Coverage   99.53%   99.53%           
=======================================
  Files          22       23    +1     
  Lines        2341     2342    +1     
=======================================
+ Hits         2330     2331    +1     
  Misses         11       11           

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

Copy link
Member

@lars-reimann lars-reimann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs automated tests. I'd suggest changing generate_stubs to return "virtual files", consisting of a Path and a string for the content.

Then handle the file I/O in _run_api_command (which should be renamed BTW).

Tests can work with the result of generate_stubs instead of having to read files from disk again.


The leading underscore should also be removed from the created file names.

@Masara Masara marked this pull request as draft April 9, 2024 20:42
@Masara Masara force-pushed the 81-file-structure-should-mirror-package-structure branch from 257c207 to 205872a Compare April 12, 2024 20:24
@Masara Masara marked this pull request as ready for review April 12, 2024 20:31
@Masara
Copy link
Contributor Author

Masara commented Apr 12, 2024

I refactored a bit here and there like you recommended and there is one test left that calls the create_stub_files function (since it also is needed for codecov). Also, I had no idea how to automate the test_file_creation test, since I'd have to read all __init__.py files of the package and compare package paths etc., which, in the end, would become way too complicated. Or is there maybe a better way, which I can't think of?

Copy link
Member

@lars-reimann lars-reimann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's still an extra folder being created:

image

Might be related to #117.

@Masara
Copy link
Contributor Author

Masara commented Apr 25, 2024

For me this seems to work correctly, did you delete the old out directory before generating new stubs? Also regarding #117, in my image.sdsstub I have the package info package safeds.data.image.containers, which also seems to be correct...

@lars-reimann
Copy link
Member

For me this seems to work correctly, did you delete the old out directory before generating new stubs? Also regarding #117, in my image.sdsstub I have the package info package safeds.data.image.containers, which also seems to be correct...

Yes, and I've checked on two devices, once with Python 3.12 and once with Python 3.11. Using the latest main of the library in both cases.

@lars-reimann
Copy link
Member

lars-reimann commented Apr 25, 2024

Using the latest main of the library in both cases.

That might be the issue: The imports in __init__ files are now in TYPE_CHECKING blocks. We've had to change that to fix the horrendous import times of the library. Maybe that requires a change?

Edit: Nevermind, I'm still getting the bugged behavior at Safe-DS/Library@e856cd5.

…e the imports names where converted too soon
@Masara
Copy link
Contributor Author

Masara commented Apr 26, 2024

I did find and fix a small bug which wouldn't let me parse the latest main, but I don't think this bug was related to your issue. Since I can't recreate this I'd say we look at it together next week.

@lars-reimann
Copy link
Member

@Masara No change in behavior, unfortunately. There's still an extra folder.

@lars-reimann
Copy link
Member

lars-reimann commented May 2, 2024

It works if an absolute path is passed as -s argument (#125):

safe-ds-stubgen -o "./out" -nc -s "E:/Repositories/safe-ds/Library/src" --docstyle NUMPYDOC -v

@lars-reimann lars-reimann merged commit ff1800e into main May 2, 2024
8 checks passed
@lars-reimann lars-reimann deleted the 81-file-structure-should-mirror-package-structure branch May 2, 2024 13:32
lars-reimann pushed a commit that referenced this pull request May 4, 2024
## [0.3.0](v0.2.0...v0.3.0) (2024-05-04)

### Features

* Added handling for sequence classes ([#127](#127)) ([cb061ab](cb061ab)), closes [#126](#126)
* DocString result names for Safe-DS stub results ([#101](#101)) ([fe163e3](fe163e3)), closes [#100](#100)
* Examples from docstrings are also taken over to stub docstrings ([#116](#116)) ([6665186](6665186)), closes [#115](#115)
* Replace the docstring_parser library with Griffe ([#79](#79)) ([9b2f802](9b2f802))

### Bug Fixes

* `Self` types as results are translated to class names  ([#110](#110)) ([4554a56](4554a56)), closes [#86](#86)
* Creating stubs with relative paths for source and output directories ([#128](#128)) ([b4493c9](b4493c9)), closes [#125](#125)
* Docstrings have the correct indentation for nested classes (stubs) ([#114](#114)) ([c7b8550](c7b8550)), closes [#113](#113)
* Fixed a bug where double ? would be generated for stubs ([#103](#103)) ([c35c6ac](c35c6ac)), closes [#87](#87) [#87](#87)
* Fixed a bug where imports would not check reexports for shortest path ([#112](#112)) ([48c5367](48c5367)), closes [#82](#82)
* Fixed a bug where results in stubs would not be named ([#131](#131)) ([4408c84](4408c84)), closes [#100](#100)
* Fixed a bug which prevented mypy version update ([#107](#107)) ([501d2cd](501d2cd))
* Fixed the stubs generator ([#108](#108)) ([9ad6df6](9ad6df6)), closes [#80](#80)
* Generated names of callback results start with result, not with param ([#104](#104)) ([6e696e9](6e696e9)), closes [#85](#85)
* Include lines of examples that start with `...` ([#130](#130)) ([3477b4a](3477b4a)), closes [#129](#129)
* No "// TODO ..." if return type is explicitly `None` ([#111](#111)) ([08e345f](08e345f)), closes [#83](#83)
* Removed the Epydoc parser ([#89](#89)) ([684a101](684a101))
* Replaced tabs with 4 spaces ([#105](#105)) ([8e7aa5d](8e7aa5d)), closes [#84](#84)
* The file structure of stubs resembles the "package" path. ([#106](#106)) ([ff1800e](ff1800e)), closes [#81](#81)
* Translation of callable ([#102](#102)) ([c581e6a](c581e6a)), closes [#88](#88) [#88](#88)
@lars-reimann
Copy link
Member

🎉 This PR is included in version 0.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@lars-reimann lars-reimann added the released Included in a release label May 4, 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.

File structure should mirror package structure
3 participants