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

dialects: Migrate func dialect to use init from get methods #1333

Merged
merged 3 commits into from Jul 26, 2023

Conversation

ShaolunWang
Copy link
Collaborator

@ShaolunWang ShaolunWang commented Jul 24, 2023

#1120: Migrate func.Call to use init from get methods

@ShaolunWang ShaolunWang added enhancement New feature or request good first issue Good for newcomers dialects Changes on the dialects labels Jul 24, 2023
@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Patch coverage: 90.90% and project coverage change: -0.01 ⚠️

Comparison is base (0401b28) 89.64% compared to head (53f4f2b) 89.63%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1333      +/-   ##
==========================================
- Coverage   89.64%   89.63%   -0.01%     
==========================================
  Files         191      191              
  Lines       24868    24873       +5     
  Branches     3750     3751       +1     
==========================================
+ Hits        22294    22296       +2     
- Misses       1982     1985       +3     
  Partials      592      592              
Impacted Files Coverage Δ
xdsl/transforms/lower_mpi.py 92.39% <ø> (ø)
xdsl/dialects/func.py 90.14% <80.00%> (-1.27%) ⬇️
tests/dialects/test_func.py 100.00% <100.00%> (ø)
xdsl/transforms/lower_snitch_runtime.py 95.74% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.


@deprecated("Use func.Call(...) instead!")
@staticmethod
def get(
Copy link
Collaborator

@compor compor Jul 24, 2023

Choose a reason for hiding this comment

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

Do we need the get now? Isn't it replaced everywhere it was referenced?

In other words, what I'm asking is when to deprecate the public API and how should we go about it in general @math-fehr ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm actually not completely sure, but I would suggest to keep it deprecated for at least one release?
I don't think we break much of the public API anymore, so I guess we should be careful.
That being said, I recall others saying that we should probably just remove it.

Copy link
Collaborator

@kingiler kingiler left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@superlopuh superlopuh left a comment

Choose a reason for hiding this comment

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

thank you

@superlopuh superlopuh merged commit 22ed220 into xdslproject:main Jul 26, 2023
10 checks passed
@ShaolunWang ShaolunWang deleted the func-init branch November 19, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialects Changes on the dialects enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants