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.

Sign up for GitHub

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

Jump to bottom

feat: Allow arguments of type pathlib.Path for file I/O methods #228

Merged
merged 10 commits into from
Apr 21, 2023
Merged

feat: Allow arguments of type pathlib.Path for file I/O methods #228

merged 10 commits into from
Apr 21, 2023

Conversation

zzril
Copy link
Contributor

@zzril zzril commented Apr 21, 2023 •
edited
Loading

Closes #146 .

Summary of Changes

Changed type hints and docstrings for

  • Table.from_csv_file
  • Table.to_csv_file
  • Table.from_json_file
  • Table.to_json_file
  • Image.from_jpeg_file
  • Image.to_jpeg_file
  • Image.from_png_file
  • Image.to_png_file
  • resolve_resource_path

to also accept arguments with type Path.

Updated tests accordingly.

Co-authored-by: robmeth [email protected]

@zzril zzril requested a review from a team as a code owner April 21, 2023 11:15
@zzril zzril linked an issue Apr 21, 2023 that may be closed by this pull request
Methods that operate on files should be able to handle a Path too #146
Closed
@lars-reimann
Copy link
Member

lars-reimann commented Apr 21, 2023 •
edited
Loading

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 8 0 0 0.83s
✅ PYTHON mypy 8 0 1.79s
✅ PYTHON ruff 8 0 0 0.04s
✅ REPOSITORY git_diff yes no 0.03s

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

@zzril zzril changed the title 146 methods that operate on files should be able to handle a path too Feat: Allow arguments of type pathlib.Path for file I/O Apr 21, 2023
@codecov
Copy link

codecov bot commented Apr 21, 2023 •
edited
Loading

Codecov Report

Merging #228 (59e0dda) into main (7f6c3bd) will decrease coverage by 0.33%.
The diff coverage is 100.00%.

❗ Current head 59e0dda differs from pull request most recent head 85c92e1. Consider uploading reports for the commit 85c92e1 to get more accurate results

@@            Coverage Diff             @@
##             main     #228      +/-   ##
==========================================
- Coverage   96.74%   96.42%   -0.33%     
==========================================
  Files          44       42       -2     
  Lines        1538     1482      -56     
==========================================
- Hits         1488     1429      -59     
- Misses         50       53       +3
Impacted Files Coverage Δ
src/safeds/data/image/containers/_image.py 100.00% <100.00%> (ø)
src/safeds/data/tabular/containers/_table.py 93.91% <100.00%> (-0.93%) ⬇️

... and 7 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@zzril zzril changed the title Feat: Allow arguments of type pathlib.Path for file I/O feat: Allow arguments of type pathlib.Path for file I/O Apr 21, 2023
zzril and others added 4 commits April 21, 2023 13:33
Fix parametrized to_json and to_csv tests
9fb27fc 
Tests were using hardcoded arguments, not the parametrized ones.
Fix docstrings
0ca8b95
Fix typehint for resolve_resource_path
fb24d9c 
Also changed typehints in docs back to "|" format.
@megalinter-bot
style: apply automated linter fixes
258a991
@zzril
Copy link
Contributor Author

zzril commented Apr 21, 2023

Note that calling the Path() constructor on a Path object simply keeps the object as is, so there's no need to add extra cases.

@zzril zzril changed the title feat: Allow arguments of type pathlib.Path for file I/O feat: Allow arguments of type pathlib.Path for file I/O methods Apr 21, 2023
zzril added 2 commits April 21, 2023 14:38
Add extra tests for saving to image given as path
a005479
Merge remote-tracking branch 'refs/remotes/origin/146-methods-that-op…
59e0dda 
…erate-on-files-should-be-able-to-handle-a-path-too' into 146-methods-that-operate-on-files-should-be-able-to-handle-a-path-too
@robmeth robmeth requested a review from lars-reimann April 21, 2023 12:49
@zzril
Copy link
Contributor Author

zzril commented Apr 21, 2023

I think it makes sense to merge #233 first, then add the Path support for the excel files in this PR.

@lars-reimann lars-reimann marked this pull request as draft April 21, 2023 14:33
@lars-reimann
Copy link
Member

I think it makes sense to merge #233 first, then add the Path support for the excel files in this PR.

Makes sense, I turned this PR into a draft again.

@lars-reimann
Copy link
Member

Since this is done and #233 still needs some work, I'll merge this in. I'd suggest that you just help out on #233 to implement the path functionality there.

@lars-reimann lars-reimann marked this pull request as ready for review April 21, 2023 18:36
@lars-reimann lars-reimann merged commit 2b58c82 into main Apr 21, 2023
@lars-reimann lars-reimann deleted the 146-methods-that-operate-on-files-should-be-able-to-handle-a-path-too branch April 21, 2023 18:40
lars-reimann pushed a commit that referenced this pull request May 11, 2023
@semantic-release-bot
chore(release): 0.12.0 [skip ci]
9189356 
## [0.12.0](v0.11.0...v0.12.0) (2023-05-11)

### Features

* add `learning_rate` to AdaBoost classifier and regressor. ([#251](#251)) ([7f74440](7f74440)), closes [#167](#167)
* add alpha parameter to `lasso_regression` ([#232](#232)) ([b5050b9](b5050b9)), closes [#163](#163)
* add parameter `lasso_ratio` to `ElasticNetRegression` ([#237](#237)) ([4a1a736](4a1a736)), closes [#166](#166)
* Add parameter `number_of_tree` to `RandomForest` classifier and regressor ([#230](#230)) ([414336a](414336a)), closes [#161](#161)
* Added `Table.plot_boxplots` to plot a boxplot for each numerical column in the table ([#254](#254)) ([0203a0c](0203a0c)), closes [#156](#156) [#239](#239)
* Added `Table.plot_histograms` to plot a histogram for each column in the table ([#252](#252)) ([e27d410](e27d410)), closes [#157](#157)
* Added `Table.transform_table` method which returns the transformed Table ([#229](#229)) ([0a9ce72](0a9ce72)), closes [#110](#110)
* Added alpha parameter to `RidgeRegression` ([#231](#231)) ([1ddc948](1ddc948)), closes [#164](#164)
* Added Column#transform ([#270](#270)) ([40fb756](40fb756)), closes [#255](#255)
* Added method `Table.inverse_transform_table` which returns the original table ([#227](#227)) ([846bf23](846bf23)), closes [#111](#111)
* Added parameter `c` to `SupportVectorMachines` ([#267](#267)) ([a88eb8b](a88eb8b)), closes [#169](#169)
* Added parameter `maximum_number_of_learner` and `learner` to `AdaBoost` ([#269](#269)) ([bb5a07e](bb5a07e)), closes [#171](#171) [#173](#173)
* Added parameter `number_of_trees` to `GradientBoosting` ([#268](#268)) ([766f2ff](766f2ff)), closes [#170](#170)
* Allow arguments of type pathlib.Path for file I/O methods ([#228](#228)) ([2b58c82](2b58c82)), closes [#146](#146)
* convert `Schema` to `dict` and format it nicely in a notebook ([#244](#244)) ([ad1cac5](ad1cac5)), closes [#151](#151)
* Convert between Excel file and `Table` ([#233](#233)) ([0d7a998](0d7a998)), closes [#138](#138) [#139](#139)
* convert containers for tabular data to HTML ([#243](#243)) ([683c279](683c279)), closes [#140](#140)
* make `Column` a subclass of `Sequence` ([#245](#245)) ([a35b943](a35b943))
* mark optional hyperparameters as keyword only ([#296](#296)) ([44a41eb](44a41eb)), closes [#278](#278)
* move exceptions back to common package ([#295](#295)) ([a91172c](a91172c)), closes [#177](#177) [#262](#262)
* precision metric for classification ([#272](#272)) ([5adadad](5adadad)), closes [#185](#185)
* Raise error if an untagged table is used instead of a `TaggedTable` ([#234](#234)) ([8eea3dd](8eea3dd)), closes [#192](#192)
* recall and F1-score metrics for classification ([#277](#277)) ([2cf93cc](2cf93cc)), closes [#187](#187) [#186](#186)
* replace prefix `n` with `number_of` ([#250](#250)) ([f4f44a6](f4f44a6)), closes [#171](#171)
* set `alpha` parameter for regularization of `ElasticNetRegression` ([#238](#238)) ([e642d1d](e642d1d)), closes [#165](#165)
* Set `column_names` in `fit` methods of table transformers to be required ([#225](#225)) ([2856296](2856296)), closes [#179](#179)
* set learning rate of Gradient Boosting models ([#253](#253)) ([9ffaf55](9ffaf55)), closes [#168](#168)
* Support vector machine for regression and for classification ([#236](#236)) ([7f6c3bd](7f6c3bd)), closes [#154](#154)
* usable constructor for `Table` ([#294](#294)) ([56a1fc4](56a1fc4)), closes [#266](#266)
* usable constructor for `TaggedTable` ([#299](#299)) ([01c3ad9](01c3ad9)), closes [#293](#293)

### Bug Fixes

* OneHotEncoder no longer creates duplicate column names ([#271](#271)) ([f604666](f604666)), closes [#201](#201)
* selectively ignore one warning instead of all warnings ([#235](#235)) ([3aad07d](3aad07d))
@lars-reimann
Copy link
Member

🎉 This PR is included in version 0.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@lars-reimann lars-reimann added the released Included in a release label May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robmeth robmeth robmeth left review comments

@lars-reimann lars-reimann lars-reimann approved these changes

Assignees
No one assigned
Labels
released Included in a release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Methods that operate on files should be able to handle a Path too
4 participants
@zzril @lars-reimann @robmeth @megalinter-bot