-
Notifications
You must be signed in to change notification settings - Fork 7
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
[WIP] Update the MkDocs for the Library Documentation #33
Conversation
def __init__(self, *, min_variance: float, name: Optional[str] = None): | ||
super().__init__(name=name) | ||
self.min_variance = min_variance | ||
self.selected_columns_ = None | ||
|
||
def partial_fit(self, X, y=None): | ||
def partial_fit(self, X:pd.Dataframe, y=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Typo in DataFrame are making the test fail
@@ -18,27 +24,20 @@ def _strided_app(a, L, S): # Window len = L, Stride len/stepsize = S | |||
return l | |||
|
|||
|
|||
def apply_rolling_data(values : np.ndarray, function, window, step=1): | |||
"""Perform a rolling window analysis at the column `col` from `data` | |||
def apply_rolling_data(values : np.ndarray, function: fun, window: int, step: int =1) -> np.array: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fun is not defined.
Maybe we can define a type with a Callable[...]
X : pd.DataFrame | ||
The input life | ||
y : [type], optional | ||
def transform(self, X: pd.DataFrame, y: Opitional[type]=None) -> pd.DataFrame: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in Optional
@@ -185,18 +210,14 @@ def __init__(self, *, feature: Optional[str] = None, name: Optional[str] = None) | |||
self.categories = set() | |||
self.encoder = None | |||
|
|||
def partial_fit(self, X: pd.DataFrame, y=None): | |||
def partial_fit(self, X: pd.DataFrame, y=None) -> SimpleEncodingCategorical: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not possible to type a method that returns a type of the class that contains it.
We can put it as string or use Self. Since python 3.11 is in typing but since we are supporting python 3.8. We need a workaround. There is a Self in the typing_extension module but i don't know the compatibility with the python version we support.
https://realpython.com/python-type-self/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I inserted the type returned by a fit or partial_fit function only for these ones in which you saw the errors. On all the other I didn't write anything. In the documentation I just inserted the input parameters because I didn't think it was essential to say that they return an object of the class (like a SimpleEncodingCategorical object in this case). But if you think we should include also this kind of information I can try to add it using the solution with Self
---------- | ||
X : pd.DataFrame | ||
The current time-series to be fitted | ||
def partial_fit(self, X: pd.DataFrame) -> IntegerIndexResamplerTransformer: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing using as type the same class name that contains it.
I know this is a WIP. I was trying to see why the test were failing and left a few comments |
Ok thanks. Tomorrow I'll fix the errors
Davide
…On Wed, Oct 18, 2023, 21:54 Luciano Lorenti ***@***.***> wrote:
I know this is a WIP. I was trying to see why the test were failing and
left a few comments
—
Reply to this email directly, view it on GitHub
<#33 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AZCZ7JDGMG34BPE4K5DXJV3YAAXWVAVCNFSM6AAAAAA6CEILHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONRZGIZDCNJVHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
In the last commit I tried to annotate the return type with Self on the partial_fit method of class SimpleEncodingCategorical. Python doesn't give me any error or warnings. Let me now if you find problems in the tests. |
Hi Davide. it seems that the tests are still failing. You can run pytest on your side to run them locally. It will catch most of the errors. Be aware that the tests in python 3.8 are saying
Because Self became part of typing in 3.11 I think. Pydantic it`s using typing-extension, so, probably also we should. |
Ok I'll try. In any case I watched the code yesterday with Francesco and
Mattia and we found out that probably instead of using Self we can use the
name of the class we created (for example TransformedDataset) in quotes (so
"TransformedDataset") and it should work. I'll try to do it in the tests
and see if it works.
…On Sat, Oct 21, 2023, 10:20 Luciano Lorenti ***@***.***> wrote:
Hi Davide. it seems that the tests are still failing. You can run pytest
on your side to run them locally. It will catch most of the errors.
Be aware that the tests in python 3.8 are saying
ImportError: cannot import name 'Self' from 'typing'
Because Self became part of typing in 3.11 I think. Pydantic it`s using
typing-extension, so, probably also we should.
https://github.com/pydantic/pydantic/blob/4dd40aec7397418e37b1029a0496b486adecbb7c/pydantic/fields.py#L217
—
Reply to this email directly, view it on GitHub
<#33 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AZCZ7JASIPAAOWDWFJOARDLYAOAT5AVCNFSM6AAAAAA6CEILHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZTG4YTKNRTHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Yes that is other possibility that for sure it will work. I want to see how other projects do it. My first try was with pydantic since they are really good at Handling types. |
Hi! |
Hi Luciano,
Unfortunately I was not able to work on Ceruleo in the last weeks. In any
case I think you can merge the actual PR and then, as soon as I can, I will
open a new PR also following the advice you gave me on how to structure the
Commit messages.
Thanks,
Davide
Il giorno gio 16 nov 2023 alle ore 09:25 Luciano Lorenti <
***@***.***> ha scritto:
… Hi!
Can we finish this and merge it?
What is there missing? Maybe the other things can be handled in other PR
—
Reply to this email directly, view it on GitHub
<#33 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AZCZ7JBZR5YYFTK57F57SGDYEXEYLAVCNFSM6AAAAAA6CEILHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJTHE4DMOJYHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Pull Request Test Coverage Report for Build 7439439342
💛 - Coveralls |
* Update mkdocs.yml * Update requirements.txt * Update extraction.md file so that it appears in the doc * Update md files to show the source code * Update docstrings in the py files * Update selection.py * Update mkdocs.yml * Update selection.py and .md * Update cast.py * Update denoising.py * Update entropy.py * Update outliers.py * Added extraction_frequency.py * Added scalers.md * Update mkdocs.yml * Update scalers.py * Transfer scaler.md to the docs/transformation/features folder * Added slicing.md and split.md * Adding operations.md and rolling_windows.md * Update operations.py and rolling_windows.py * Update operations.py and rolling_windows.py * Added transformation.md * Update transformation.py * Update mkdocs.yml * Update transformation.py --------- Co-authored-by: Lemeda98 <lemeda98@gmail.com> Co-authored-by: Luciano Lorenti <lucianolorenti@gmail.com>
No description provided.