Skip to content

Use DEDUPLICATION_TAG to determine whether a citation node is in a docstring #179

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

Merged
merged 4 commits into from
Apr 8, 2019

Conversation

jnothman
Copy link
Member

@jnothman jnothman commented Jun 6, 2018

Fixes #177

This should handle the case of a module docstring when relabelling munged reference labels.

This probably deserves a non-regression test.

@jnothman
Copy link
Member Author

@prisae says this works for him, which is a good start!

@prisae
Copy link

prisae commented Jun 27, 2018

Thanks @jnothman, would indeed have been better to state it here.

@prisae
Copy link

prisae commented Dec 10, 2018

Hi @jnothman. Do you know if there is any new release planned in the near future (so these changes would be available, for instance, in readthedocs.org)?

@jnothman
Copy link
Member Author

Sorry, I've not given this project much attention (along with several others I have not had time to give to). This would need to be reviewed by other contributors and merged before release.

@rgommers rgommers added this to the v0.9.0 milestone Dec 11, 2018
@rgommers
Copy link
Member

I don't have plans for a release in the near future, sorry, too many other balls to juggle at the moment. Unless something breaks or gets fixed that ups the priority.

@prisae
Copy link

prisae commented Dec 11, 2018

Thanks @jnothman and @rgommers . No problem, just thought I'd ask. Might do it again in a couple of months... 😉

@jnothman
Copy link
Member Author

jnothman commented Apr 2, 2019

This has been road tested as noted above.

@prisae
Copy link

prisae commented Apr 2, 2019

Yes, I can use it locally without issues. However, if it is released it would be possible to use it, e.g., on readthedocs.io etc. (There are probably workarounds to install directly from GitHub instead of pypi on readthedocs and friends.)

@larsoner
Copy link
Collaborator

larsoner commented Apr 2, 2019

@jnothman is it easy enough to add a regression test? If not perhaps we can live with the user tests here

@larsoner
Copy link
Collaborator

larsoner commented Apr 2, 2019

There are probably workarounds to install directly from GitHub instead of pypi on readthedocs and friends

pip will accept things like:

$ pip install https://api.github.com/repos/numpy/numpydoc/zipball/master

And such entries can live in requirements.txt files

@prisae
Copy link

prisae commented Apr 2, 2019

Thanks @larsoner, will give that a try!

@rgommers
Copy link
Member

rgommers commented Apr 5, 2019

Looks like we should merge this for the next release, even without a test.

@jnothman agree? can you rebase?

@jnothman
Copy link
Member Author

jnothman commented Apr 6, 2019

Happy to do the rebase. I haven't reassessed how easily it can be tested.

@larsoner
Copy link
Collaborator

larsoner commented Apr 8, 2019

Works fine for me on MNE and SciPy docs, but neither of those showed the problem.

@prisae can you test this PR to see if it fixes your problem?

@prisae
Copy link

prisae commented Apr 8, 2019

I did, and it works fine for me (#177 (comment)). Locally I am using it since @jnothman published it, without issues.

@larsoner
Copy link
Collaborator

larsoner commented Apr 8, 2019

Okay let's merge for now then since it seems to work, not break anything, and @rgommers is happy (and so am I). Thanks @jnothman

@larsoner larsoner merged commit c8513c5 into numpy:master Apr 8, 2019
@prisae
Copy link

prisae commented Apr 8, 2019

Great, happy to have that in, thanks everyone!

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

Successfully merging this pull request may close these issues.

4 participants