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

Investigate need for #1720 #1721

Closed
jonmmease opened this issue Aug 20, 2021 · 4 comments
Closed

Investigate need for #1720 #1721

jonmmease opened this issue Aug 20, 2021 · 4 comments

Comments

@jonmmease
Copy link
Contributor

Investigate need for #1720 and see if there's a way to remove the _pytest part from the Dash constructor.

@jonmmease jonmmease self-assigned this Aug 20, 2021
@jonmmease
Copy link
Contributor Author

I spent a while playing with this, but didn't get too for. I don't think the current approach is causing any problems, so I'm not going to dig into it much more. But @alexcjohnson may want to have a look when he has a chance.

@jonmmease jonmmease removed their assignment Aug 20, 2021
@alexcjohnson
Copy link
Collaborator

Thanks for pointing this out @jonmmease

Just to be clear: this code was added during #1679 in commit e169cbc and then scoped more tightly in #1720, right? @HammadTheOne can you explain what prompted you to add this? Do you understand why it was only needed in the monorepo context but not before that?

@HammadTheOne
Copy link
Contributor

this code was added during #1679 in commit e169cbc and then scoped more tightly in #1720, right?

Yes, that's correct. In my understanding, prior to moving to the monorepo and serving dependencies from within dash, packages is a list of module loaders for whichever packages are in ComponentRegistry, namely dcc, html, table. Now that these packages are within the dash namespace, when Pytest is running a test app, it the assertion rewriter is the loader for dash (and I'm not entirely sure why that is in this context, but it has to do with the way it handles assertions).

As a workaround for this object being passed from pytest, I added this conditional to check if the package was an instance of AssertionRewritingHook and to set the correct path/location for the dash package, so that devtools/hot reloading would work as normal with a pytest configuration.

@gvwilson
Copy link
Contributor

Hi - we are tidying up stale issues and PRs in Plotly's public repositories so that we can focus on things that are most important to our community. If this issue is still a concern, please add a comment letting us know what recent version of our software you've checked it with so that I can reopen it and add it to our backlog. (Please note that we will give priority to reports that include a short reproducible example.) If you'd like to submit a PR, we'd be happy to prioritize a review, and if it's a request for tech support, please post in our community forum. Thank you - @gvwilson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants